Release 0.10.x Checking Coderabbit AI #456
Open
Conversation
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
Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> Signed-off-by: Vishwa <visu.vs1@gmail.com>
Signed-off-by: Abhishek S <127825992+abhishek8shankar@users.noreply.github.com>
Signed-off-by: Abhishek S <127825992+abhishek8shankar@users.noreply.github.com>
[MOSIP-39240] Updated install.sh and values.yaml to use config-server
Signed-off-by: Mohanachandran S <mohanachandran.s@technoforte.co.in>
* Openapi review comments Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * [Release Changes] Updated openapi docs Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * [release changes] updated openapi docs Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * [Swagger changes] added additional examples Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * [Openapi changes] updated scope Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * [Updated README] added rendering template workflow in readme Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * [Restructured readme] Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * [cleanup] Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * [removed unnecessary headers] Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> * removed unnecessary content Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> --------- Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com> Signed-off-by: Vishwa <visu.vs1@gmail.com>
Signed-off-by: techno-467 <prafulrakhade02@gmail.com>
Signed-off-by: techno-467 <prafulrakhade02@gmail.com>
Signed-off-by: techno-467 <prafulrakhade02@gmail.com>
[DSD-7171] inji certify 0.10.0 release
Signed-off-by: Abhishek S <127825992+abhishek8shankar@users.noreply.github.com>
Signed-off-by: Rakshitha650 <76676196+Rakshitha650@users.noreply.github.com>
Signed-off-by: GitHub <noreply@github.com> Co-authored-by: Prafulrakhade <Prafulrakhade@users.noreply.github.com>
* Updated Pom version and Chart version for post-release changes Signed-off-by: GitHub <noreply@github.com> * [DSD-7203] Update docker-compose.yaml Signed-off-by: Praful Rakhade <prafulrakhade02@gmail.com> * [DSD-7203] Updated values.yaml Signed-off-by: Praful Rakhade <prafulrakhade02@gmail.com> * [DSD-7203] Update install.sh Signed-off-by: Praful Rakhade <prafulrakhade02@gmail.com> * DSD-7203 inji certify 0.10.1 release Signed-off-by: techno-467 <prafulrakhade02@gmail.com> --------- Signed-off-by: GitHub <noreply@github.com> Signed-off-by: Praful Rakhade <prafulrakhade02@gmail.com> Signed-off-by: techno-467 <prafulrakhade02@gmail.com> Co-authored-by: Prafulrakhade <Prafulrakhade@users.noreply.github.com> Co-authored-by: Praful Rakhade <prafulrakhade02@gmail.com>
Signed-off-by: GitHub <noreply@github.com> Co-authored-by: Prafulrakhade <Prafulrakhade@users.noreply.github.com>
* Updated Pom version and Chart version for post-release changes Signed-off-by: GitHub <noreply@github.com> * [DSD-7203] inji-certify 0.10.1 post release Signed-off-by: Praful Rakhade <prafulrakhade02@gmail.com> --------- Signed-off-by: GitHub <noreply@github.com> Signed-off-by: Praful Rakhade <prafulrakhade02@gmail.com> Co-authored-by: Prafulrakhade <Prafulrakhade@users.noreply.github.com> Co-authored-by: Praful Rakhade <prafulrakhade02@gmail.com>
Signed-off-by: Harsh Vardhan <harsh59v@gmail.com>
* [INJICERT-898] update version for patch release Signed-off-by: Harsh Vardhan <harsh59v@gmail.com> * [INJICERT-898] bump up api-test version Signed-off-by: Harsh Vardhan <harsh59v@gmail.com> --------- Signed-off-by: Harsh Vardhan <harsh59v@gmail.com>
Signed-off-by: Abhi <abhishek.shankarcs@gmail.com>
Signed-off-by: GitHub <noreply@github.com> Co-authored-by: Prafulrakhade <Prafulrakhade@users.noreply.github.com>
Signed-off-by: Hitesh Jain <jainhitesh9998@gmail.com>
* Updated chart versions, image and tag for release changes Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * [DSD-] Signed-off-by: techno-467 <prafulrakhade02@gmail.com> * [DSD-7420] inji certify 0.10.2 release Signed-off-by: techno-467 <prafulrakhade02@gmail.com> * [DSD-7420] inji certify 0.10.2 release Signed-off-by: techno-467 <prafulrakhade02@gmail.com> --------- Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: techno-467 <prafulrakhade02@gmail.com> Co-authored-by: Prafulrakhade <99539100+Prafulrakhade@users.noreply.github.com> Co-authored-by: techno-467 <prafulrakhade02@gmail.com>
* Updated Pom version and Chart version for post-release changes Signed-off-by: GitHub <noreply@github.com> * Updated install.sh Signed-off-by: Praful Rakhade <prafulrakhade02@gmail.com> --------- Signed-off-by: GitHub <noreply@github.com> Signed-off-by: Praful Rakhade <prafulrakhade02@gmail.com> Co-authored-by: Prafulrakhade <Prafulrakhade@users.noreply.github.com> Co-authored-by: Praful Rakhade <prafulrakhade02@gmail.com>
WalkthroughRemoves a CI Sonar job; updates docs (README, rendering template guide, OpenAPI spec); bumps versions across Maven modules, Helm chart/values, Docker images; adjusts deployment scripts and config server naming; modifies init container SSL handling; updates DB init values and adds no-op upgrade/rollback scripts; tweaks API test templates and skip list. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as Operator
participant H as Helm
participant K as Kubernetes
participant CS as config-server (Deployment)
participant CM as config-server-share (ConfigMap)
U->>H: helm install certify --version 0.10.2-develop
H->>K: Create resources (Deployments, ConfigMaps, InitContainers)
Note over CS: Init: SSL cert fetch and import into cacerts
K->>CM: Mount extra env from config-server-share
U->>K: kubectl set env deployment/config-server ... OVERRIDES_*
K->>CS: Apply updated env vars
CS-->>U: Running with updated overrides
sequenceDiagram
autonumber
participant Wallet as Mobile Wallet
participant Issuer as Credential Issuer
participant Render as Rendering Service
participant Store as Template Store
Wallet->>Issuer: GET /.well-known/openid-credential-issuer
Issuer-->>Wallet: Metadata (includes renderMethod URL)
Wallet->>Render: GET /rendering-templates/{id}
Render->>Store: Load template (cache if present)
Store-->>Render: Template SVG/JSON
Render-->>Wallet: 200 OK (SVG/JSON + cache headers)
Wallet->>Issuer: POST /credential (CredentialRequest)
Issuer-->>Wallet: Credential (per requested format)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
helm/inji-certify/values.yaml (3)
245-255: Fix variable name bug and make the script deterministic.
- Bug: uses
$hostinstead of$HOSTin the error message.- Remove background
openssl+sleep 2; read synchronously and extract cert reliably.- - if [ "$ENABLE_INSECURE" = "true" ]; then HOST=$( env | grep "mosip-api-internal-host" |sed "s/mosip-api-internal-host=//g"); if [ -z "$HOST" ]; then echo "HOST $HOST is empty; EXITING"; exit 1; fi; openssl s_client -servername "$HOST" -connect "$HOST":443 > "$HOST.cer" 2>/dev/null & sleep 2 ; sed -i -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' "$HOST.cer"; cat "$HOST.cer"; /usr/local/openjdk-11/bin/keytool -delete -alias "$HOST" -keystore $JAVA_HOME/lib/security/cacerts -storepass changeit; /usr/local/openjdk-11/bin/keytool -trustcacerts -keystore "$JAVA_HOME/lib/security/cacerts" -storepass changeit -noprompt -importcert -alias "$HOST" -file "$HOST.cer" ; if [ $? -gt 0 ]; then echo "Failed to add SSL certificate for host $host; EXITING"; exit 1; fi; cp /usr/local/openjdk-11/lib/security/cacerts /cacerts; fi + - if [ "$ENABLE_INSECURE" = "true" ]; then HOST=$(env | grep "^mosip-api-internal-host=" | sed "s/mosip-api-internal-host=//"); if [ -z "$HOST" ]; then echo "HOST is empty; EXITING"; exit 1; fi; \ + openssl s_client -servername "$HOST" -connect "$HOST:443" -showcerts </dev/null 2>/dev/null | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' > "$HOST.cer"; \ + /usr/local/openjdk-11/bin/keytool -delete -alias "$HOST" -keystore "$JAVA_HOME/lib/security/cacerts" -storepass changeit >/dev/null 2>&1 || true; \ + /usr/local/openjdk-11/bin/keytool -trustcacerts -keystore "$JAVA_HOME/lib/security/cacerts" -storepass changeit -noprompt -importcert -alias "$HOST" -file "$HOST.cer"; \ + if [ $? -ne 0 ]; then echo "Failed to add SSL certificate for host $HOST; EXITING"; exit 1; fi; \ + cp /usr/local/openjdk-11/lib/security/cacerts /cacerts; fi
246-251: Don’t default to insecure; wire to values.enable_insecure.Default ENABLE_INSECURE to “false” and template from
.Values.enable_insecure(which is already false at line 406) to avoid surprising trust changes.- env: - - name: ENABLE_INSECURE - value: "true" + env: + - name: ENABLE_INSECURE + value: "{{ .Values.enable_insecure | default false }}"
347-351: Align PrometheusendpointPathwith the management base path
Probes target/v1/certify/actuator/healthwhileendpointPathis set to/v1/inji-certify/actuator/prometheus; update line 349 in helm/inji-certify/values.yaml to/v1/certify/actuator/prometheus(or adjust both paths to your actual context) to prevent scrape failures.docs/inji-certify-openapi.yaml (2)
191-354: Replace schema fragments inside x-examples with concrete example objects.Under “Example vd11” you embed OpenAPI schema keywords (type/properties/items) instead of an instance. This breaks example validity and confuses tooling.
I can help convert these to concrete JSON example payloads matching the described schema.
1300-1301: Add security to the generic /issuance/credential endpoint for parity.vd11/vd12 endpoints declare bearer security; /issuance/credential should, too.
/issuance/credential: post: summary: Credential Issuance endpoint description: 'Once the access token is received via the token endpoint, Wallet should invoke this endpoint to get the verifiable credential.' operationId: post-vci-credential + security: + - Authorization-access_token: [] requestBody:Also applies to: 1081-1142
deploy/inji-certify-apitestrig/install.sh (3)
55-67: Fix flag prompt logic and accept y/Y/n/N.Current message says “‘flag’ was provided” on empty input; also only ‘n’ is handled.
-read -p "" flag +read -r flag if [ -z "$flag" ]; then - echo "'flag' was provided; EXITING;" + echo "No selection provided; EXITING." exit 1; fi ENABLE_INSECURE='' -if [ "$flag" = "n" ]; then +flag=${flag,,} # normalize case +if [ "$flag" = "n" ]; then ENABLE_INSECURE='--set enable_insecure=true'; fi ``` <!-- review_comment_end --> --- `79-85`: **Quote variable in -z test to avoid unary-operator errors.** Unquoted empty value leads to a parsing error and the check is skipped. ```diff -read -p "Please provide slack webhook URL to notify server end issues on your slack channel : " slackWebhookUrl +read -r -p "Please provide Slack webhook URL to notify server issues on your Slack channel: " slackWebhookUrl - -if [ -z $slackWebhookUrl ]; then +if [ -z "$slackWebhookUrl" ]; then echo "slack webhook URL not provided; EXITING;" exit 1; fi ``` <!-- review_comment_end --> --- `20-27`: **Typos and stray echo.** “Secrtes” → “Secrets”; remove duplicate “echo”. ```diff - echo Copy Configmaps + echo Copy ConfigMaps ... - echo echo Copy Secrtes + echo Copy Secrets ``` <!-- review_comment_end --> </blockquote></details> </blockquote></details>🧹 Nitpick comments (21)
api-test/README.md (1)
206-208: Fix Markdown list indentation (MD007).
Adjust indentation for nested bullets under “View Test Results”.- - The report will have two sections: - - One section for **pre-requisite APIs** test cases. - - Another section for **core test cases**. +- The report will have two sections: + - One section for **pre-requisite APIs** test cases. + - Another section for **core test cases**.docker-compose/docker-compose-injistack/docker-compose.yaml (1)
17-17: Approve mosipid image updates; confirm tag existence and compatibility.No stale
mosipqa/references remain in compose files. Please verify thatmosipid/inji-certify:0.10.2,mosipid/inji-mimoto:0.15.0, andmosipid/inji-web:0.11.0exist in the registry and are mutually compatible for local workflows. Optional: pin the Postgres service to a specific version (e.g.,postgres:16.3) and addrestart: unless-stoppedfor improved resilience.docker-compose/docker-compose-injistack/README.md (3)
59-63: Specify code-block language and clarify “latest” vs pinned JAR.
- Add a language hint to satisfy markdownlint and improve rendering.
- “Download the latest JAR…” but the link pins 0.3.0. Either:
- say “Download version 0.3.0” or
- link to the artifact directory and note “pick the latest”.
Example:
-- Download the latest JAR from the below link and place the downloaded JAR in `loader_path/certify/` +– Download the JAR (version 0.3.0 shown below) and place it in `loader_path/certify/` + +```text +https://repo1.maven.org/maven2/io/mosip/certify/mock-certify-plugin/0.3.0/mock-certify-plugin-0.3.0.jar +``` - ``` - https://repo1.maven.org/maven2/io/mosip/certify/mock-certify-plugin/0.3.0/mock-certify-plugin-0.3.0.jar - ```
1-3: Fix high-visibility typos in the opening paragraph.“it's” → “its”, “deparments” → “departments”, “accelarate” → “accelerate”. Improves professionalism without changing meaning.
165-166: Typo: “Verfier” → “Verifier”.Update the link text: “Standards Compliant VC Verifier”.
README.md (2)
19-21: Add fenced code language for the URL block.Use “text” for a bare URL to satisfy MD040:
- ``` - https://repo1.maven.org/maven2/io/mosip/certify/postgres-dataprovider-plugin/0.3.0/postgres-dataprovider-plugin-0.3.0.jar - ``` + ```text + https://repo1.maven.org/maven2/io/mosip/certify/postgres-dataprovider-plugin/0.3.0/postgres-dataprovider-plugin-0.3.0.jar + ```
54-55: Fix unordered list indentation (MD007).Indent the “Certify” bullet with two spaces to match the list level.
helm/inji-certify/values.yaml (1)
249-255: Avoid running the init container as root.Instead of modifying the system cacerts and copying it, create/import directly into the mounted volume (e.g.,
/cacerts/cacerts) and keep a non-root UID. This reduces attack surface while achieving the same outcome.Would you like a patched command that writes a fresh keystore to
/cacerts/cacertswithout touching$JAVA_HOME?docs/Rendering-Template.md (5)
54-56: Fix heading level increment (MD001).Jumping from H1 to H3. Use H2.
-### 🔄 Credential Flow +## 🔄 Credential Flow ``` <!-- review_comment_end --> --- `70-76`: **Add Vary header to the table (doc mentions it earlier).** Keep docs and API spec consistent. ```diff | Header | Value | Purpose | |--------|-------|---------| | Content-Type | image/svg+xml | MIME type enforcement | | Cache-Control | max-age=604800 | CDN/browser caching | +| Vary | Accept-Language | Serve locale-specific templates correctly | ``` <!-- review_comment_end --> --- `50-51`: **Align Cache-Control duration across docs and OpenAPI.** Docs say 604800 (7 days) while OpenAPI example uses 2592000 (30 days). Pick one policy and apply everywhere. <!-- review_comment_end --> Also applies to: 75-76 --- `77-77`: **Clarify digestMultibase guidance and grammar.** Tighten grammar and remove ambiguity about when to hardcode vs evaluate. ```diff -The digest multibase can be hardcoded or if the template has been stored with Certify's DB & `mosip.certify.data-provider-plugin.rendering-template-id` is set to the correct the value `${_renderMethodSVGdigest}` can be used to enable Certify to evaluate it specifying the id of the rendering-template used. However, for optimal performance, it's recommended to not set this key and instead hardcode the `digestMultibase` value in the Velocity template itself. +The `digestMultibase` MAY be: +- hardcoded in the Velocity template (recommended for performance and cacheability), or +- resolved by Certify from DB when `mosip.certify.data-provider-plugin.rendering-template-id` is set (use `${_renderMethodSVGdigest}` in the template). ``` <!-- review_comment_end --> --- `46-52`: **Add a brief security note for SVG delivery.** SVG can contain active content. Recommend CSP/sanitization guidance. Add a note: “Serve SVG with strict CSP (e.g., disallow script), set X-Content-Type-Options: nosniff, and sanitize templates to prevent active content execution.” <!-- review_comment_end --> </blockquote></details> <details> <summary>docs/inji-certify-openapi.yaml (3)</summary><blockquote> `668-752`: **“Success response” example: ensure fields mirror schema and remove residual schema-like bits.** Looks fine largely; double-check that property names align (e.g., credential_configurations_supported vs credentials_supported for the chosen draft). <!-- review_comment_end --> --- `883-896`: **Add Vary: Accept-Language header for SVG templates.** The docs describe Vary; add it here for alignment and correct caching behavior. ```diff headers: Cache-Control: description: Caching policy for the SVG template. schema: type: string example: public, max-age=2592000 + Vary: + description: Indicates the response varies by language. + schema: + type: string + example: Accept-Language ``` <!-- review_comment_end --> --- `17-27`: **Version enum doesn’t include vd13 while examples reference it.** Either add vd13 to the enum or document that latest maps to vd13 and remove the vd13-labeled example to avoid confusion. <!-- review_comment_end --> Also applies to: 516-523 </blockquote></details> <details> <summary>db_upgrade_script/mosip_certify/sql/0.10.0_to_0.10.1_rollback.sql (1)</summary><blockquote> `1-1`: **Add a brief header comment for operators.** Clarify that this rollback is a no-op and relies on external env vars. ```diff +-- No-op rollback: 0.10.0 → 0.10.1 requires no DB changes. +-- Expects $CURRENT_VERSION and $UPGRADE_VERSION to be set by the runner. \echo 'Rollback Queries not required for transition from $CURRENT_VERSION to $UPGRADE_VERSION' ``` <!-- review_comment_end --> </blockquote></details> <details> <summary>deploy/inji-certify-apitestrig/install.sh (3)</summary><blockquote> `41-53`: **Robust numeric validation and clearer error messages for time input.** Use regex and consistent messages; current check `if ! [ $time -eq $time ]` is brittle. ```diff -read -p "Please enter the time(hr) to run the cronjob every day (time: 0-23) : " time +read -r -p "Please enter the hour (0-23) to run the cronjob each day: " time if [ -z "$time" ]; then - echo "ERROT: Time cannot be empty; EXITING;"; + echo "ERROR: Time cannot be empty; EXITING."; exit 1; fi -if ! [ $time -eq $time ] 2>/dev/null; then - echo "ERROR: Time $time is not a number; EXITING;"; +if ! [[ "$time" =~ ^[0-9]+$ ]]; then + echo "ERROR: Time '$time' is not a number; EXITING."; exit 1; fi -if [ $time -gt 23 ] || [ $time -lt 0 ] ; then +if [ "$time" -gt 23 ] || [ "$time" -lt 0 ] ; then echo "ERROR: Time should be in range ( 0-23 ); EXITING;"; exit 1; fi ``` <!-- review_comment_end --> --- `12-14`: **Namespace creation should be idempotent under set -e.** Use apply-style creation to avoid failure if NS exists. ```diff -echo Create $NS namespace -kubectl create ns $NS +echo "Ensure namespace $NS exists" +kubectl get ns "$NS" >/dev/null 2>&1 || kubectl create ns "$NS" ``` <!-- review_comment_end --> --- `37-40`: **Pre-flight dependency checks (jq, helm repo).** Script assumes jq and ‘mosip’ helm repo exist; add guard rails. Add: ```bash command -v jq >/dev/null || { echo "jq not found; EXITING."; exit 1; } helm repo list | grep -q '^mosip' || helm repo add mosip https://mosip.github.io/mosip-helm ``` <!-- review_comment_end --> </blockquote></details> <details> <summary>helm/inji-certify/Chart.yaml (1)</summary><blockquote> `5-6`: **Set appVersion to the image/tag version for clarity.** Helm UIs surface appVersion; helps users map chart to app release. ```diff -version: 0.10.2-develop -appVersion: "" +version: 0.10.2-develop +appVersion: "0.10.2" ``` <!-- review_comment_end --> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 03a2a73a7a307b0a608b0e5a7a234e5398a4c838 and 401a8805227de1fea9e554abcff329aa32f7f01b. </details> <details> <summary>📒 Files selected for processing (25)</summary> * `.github/workflows/push-trigger.yml` (0 hunks) * `README.md` (3 hunks) * `api-test/README.md` (4 hunks) * `api-test/pom.xml` (2 hunks) * `api-test/src/main/resources/injicertify/AddIdentityMock/AddIdentity.hbs` (0 hunks) * `api-test/src/main/resources/testCaseSkippedList.txt` (1 hunks) * `certify-core/pom.xml` (1 hunks) * `certify-integration-api/pom.xml` (1 hunks) * `certify-service/pom.xml` (1 hunks) * `db_scripts/init_db.sh` (1 hunks) * `db_scripts/init_values.yaml` (2 hunks) * `db_upgrade_script/mosip_certify/sql/0.10.0_to_0.10.1_rollback.sql` (1 hunks) * `db_upgrade_script/mosip_certify/sql/0.10.0_to_0.10.1_upgrade.sql` (1 hunks) * `db_upgrade_script/mosip_certify/sql/0.10.1_to_0.10.2_rollback.sql` (1 hunks) * `db_upgrade_script/mosip_certify/sql/0.10.1_to_0.10.2_upgrade.sql` (1 hunks) * `deploy/inji-certify-apitestrig/install.sh` (1 hunks) * `deploy/inji-certify-apitestrig/values.yaml` (1 hunks) * `deploy/inji-certify/install.sh` (2 hunks) * `docker-compose/docker-compose-injistack/README.md` (1 hunks) * `docker-compose/docker-compose-injistack/docker-compose.yaml` (3 hunks) * `docs/Rendering-Template.md` (1 hunks) * `docs/inji-certify-openapi.yaml` (21 hunks) * `helm/inji-certify/Chart.yaml` (1 hunks) * `helm/inji-certify/values.yaml` (3 hunks) * `pom.xml` (2 hunks) </details> <details> <summary>💤 Files with no reviewable changes (2)</summary> * .github/workflows/push-trigger.yml * api-test/src/main/resources/injicertify/AddIdentityMock/AddIdentity.hbs </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docker-compose/docker-compose-injistack/README.md</summary> 61-61: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/Rendering-Template.md</summary> 54-54: Heading levels should only increment by one level at a time Expected: h2; Actual: h3 (MD001, heading-increment) </details> <details> <summary>api-test/README.md</summary> 206-206: Unordered list indentation Expected: 0; Actual: 3 (MD007, ul-indent) --- 207-207: Unordered list indentation Expected: 2; Actual: 7 (MD007, ul-indent) --- 208-208: Unordered list indentation Expected: 2; Actual: 7 (MD007, ul-indent) </details> <details> <summary>README.md</summary> 54-54: Unordered list indentation Expected: 2; Actual: 3 (MD007, ul-indent) </details> </details> </details> <details> <summary>🔇 Additional comments (14)</summary><blockquote> <details> <summary>certify-integration-api/pom.xml (1)</summary><blockquote> `9-9`: **Parent version bump aligned—LGTM.** No concerns; inherits `${project.parent.version}` correctly. </blockquote></details> <details> <summary>pom.xml (2)</summary><blockquote> `18-18`: **Parent version set to 0.10.2—LGTM.** Consistent with module POMs. --- `103-103`: **pom.xml:103 — Verify kernel.keymanager.version (1.3.0-beta.2) vs kernel.core.version (1.3.0-beta.1).** Ensure no API incompatibilities or transitive-dependency overrides by running `mvn dependency:tree` and a full build/tests locally. </blockquote></details> <details> <summary>certify-core/pom.xml (1)</summary><blockquote> `13-13`: **Parent version bump aligned—LGTM.** No further action. </blockquote></details> <details> <summary>db_scripts/init_db.sh (1)</summary><blockquote> `10-10`: **Remove invalid version warning** `0.0.1-develop` is listed in the mosip/postgres-init Helm repository index and is a valid chart version. > Likely an incorrect or invalid review comment. </blockquote></details> <details> <summary>certify-service/pom.xml (1)</summary><blockquote> `12-12`: **Version bump to 0.10.2 confirmed across all modules; no SNAPSHOT references remain—approved.** </blockquote></details> <details> <summary>deploy/inji-certify-apitestrig/values.yaml (1)</summary><blockquote> `5-6`: **Image bump and cross‐chart alignment verified** Helm charts (inji-certify & apitestrig) values.yaml and the deploy/install script consistently reference mosipid and image tag 0.10.2. </blockquote></details> <details> <summary>api-test/pom.xml (1)</summary><blockquote> `11-11`: **Version alignment looks good; confirm artifact consumers use the new shaded JAR name.** POM version and fileName updated to 0.10.2; dependency pinned to 1.3.1. Ensure any scripts/jobs copying “apitest-injicertify-*-jar-with-dependencies.jar” are updated if they used the exact filename. Also applies to: 54-55, 61-61 </blockquote></details> <details> <summary>helm/inji-certify/values.yaml (1)</summary><blockquote> `219-221`: **Ensure new ConfigMap name is in place and available** - Chart now references only `config-server-share`; no occurrences of `inji-config-server-share` remain. - Manually confirm that the `config-server-share` ConfigMap exists in all target clusters before performing the upgrade. </blockquote></details> <details> <summary>docs/inji-certify-openapi.yaml (1)</summary><blockquote> `1089-1090`: **CredentialRequest: good centralization via $ref.** Nice refactor to reuse the request schema across versions. <!-- review_comment_end --> Also applies to: 1215-1217, 1309-1311 </blockquote></details> <details> <summary>deploy/inji-certify/install.sh (3)</summary><blockquote> `16-16`: **Manual verification required: Helm not installed in sandbox** The CI environment cannot verify `mosip/inji-certify` version automatically. Please run locally: ```shell helm repo add mosip https://mosip.github.io/mosip-helm helm repo update helm search repo mosip/inji-certify --versions | grep 0.10.2-developand confirm that
0.10.2-developis published before merging.
10-10: Confirm Softhsm chart tag exists
Sandbox lacks Helm CLI; please manually verify that1.3.0-beta.2is published in the mosip Helm repository (e.g.,helm search repo mosip/softhsm --versions).
43-44: Manual verification required for hyphenated env var names and ConfigMap namespace existenceThe sandbox lacks
kubectl, so please confirm that using keys with hyphens inkubectl set envproduces invalid environment variable names and verify that theglobalConfigMap is present in theconfig-servernamespace.db_upgrade_script/mosip_certify/sql/0.10.1_to_0.10.2_rollback.sql (1)
1-1: Replace shell‐style variables with psql variables in\echo
\echo '…$CURRENT_VERSION…$UPGRADE_VERSION'prints the literals; use psql variables instead:\echo Rollback Queries not required for transition from :CURRENT_VERSION to :UPGRADE_VERSION ``` ([docs.huihoo.com](https://docs.huihoo.com/redhat/database/rhdb-2.0/admin/html/r1-app-psql-4.html?utm_source=openai))- Or, for a client-agnostic SQL alternative:
SELECT 'Rollback Queries not required for transition from ' || current_setting('app.current_version', true) || ' to ' || current_setting('app.upgrade_version', true) || '.' AS info;Ensure those GUCs are set in your upgrade driver.
| 2. Run the automation test suite JAR file: | ||
| ``` | ||
| java -jar -Dmodules=injicertify -Denv.user=api-internal.<env_name> -Denv.endpoint=<base_env> -Denv.testLevel=smokeAndRegression -jar apitest-injicertify-1.3.0-SNAPSHOT-jar-with-dependencies.jar | ||
| java -jar -Dmodules=injicertify -Denv.user=api-internal.<env_name> -Denv.endpoint=<base_env> -Denv.testLevel=smokeAndRegression -jar apitest-injicertify-0.10.2-SNAPSHOT-jar-with-dependencies.jar |
There was a problem hiding this comment.
Use released JAR name (drop -SNAPSHOT) to match 0.10.2.
Command should reference the released artifact to avoid confusion.
- java -jar -Dmodules=injicertify -Denv.user=api-internal.<env_name> -Denv.endpoint=<base_env> -Denv.testLevel=smokeAndRegression -jar apitest-injicertify-0.10.2-SNAPSHOT-jar-with-dependencies.jar
+ java -jar -Dmodules=injicertify -Denv.user=api-internal.<env_name> -Denv.endpoint=<base_env> -Denv.testLevel=smokeAndRegression -jar apitest-injicertify-0.10.2-jar-with-dependencies.jar📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| java -jar -Dmodules=injicertify -Denv.user=api-internal.<env_name> -Denv.endpoint=<base_env> -Denv.testLevel=smokeAndRegression -jar apitest-injicertify-0.10.2-SNAPSHOT-jar-with-dependencies.jar | |
| java -jar -Dmodules=injicertify -Denv.user=api-internal.<env_name> -Denv.endpoint=<base_env> -Denv.testLevel=smokeAndRegression -jar apitest-injicertify-0.10.2-jar-with-dependencies.jar |
🤖 Prompt for AI Agents
In api-test/README.md around line 156, the example command references the
snapshot artifact
'apitest-injicertify-0.10.2-SNAPSHOT-jar-with-dependencies.jar'; update the
command to use the released artifact by removing '-SNAPSHOT' so it matches
version 0.10.2 (e.g., apitest-injicertify-0.10.2-jar-with-dependencies.jar) and
ensure any accompanying documentation text reflects the released JAR name.
Comment on lines
+2
to
+4
| #MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_PosINJICERT-681 | ||
| INJICERT-681------InjiCertify_GetCredentialForMockIDA_IdpAccessToken_Invalid_Format_Neg | ||
| INJICERT-681------InjiCertify_GetCredentialForMockIDA_IdpAccessToken_MoreThanOne_Format_Neg |
There was a problem hiding this comment.
🧩 Analysis chain
Split concatenated entries; likely parsing bug.
Current line mixes two items: “...smoke_PosINJICERT-681”. Suggest:
-#MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_PosINJICERT-681
+#MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_Pos
+INJICERT-681------InjiCertify_GetCredentialForMockIDA_IdpAccessToken_Invalid_Format_NegRun a quick check to ensure each line is a distinct test ID (no spaces or accidental joins).
🏁 Script executed:
#!/bin/bash
awk 'length($0)>0{ if($0 ~ /PosINJICERT/) { print "Possible join:", NR, $0 } }' api-test/src/main/resources/testCaseSkippedList.txtLength of output: 83
Remove concatenated test ID in api-test/src/main/resources/testCaseSkippedList.txt
Line 2 merges two test IDs; split them by dropping the trailing “INJICERT-681” so each entry is on its own line.
-#MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_PosINJICERT-681
+#MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_Pos📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_PosINJICERT-681 | |
| INJICERT-681------InjiCertify_GetCredentialForMockIDA_IdpAccessToken_Invalid_Format_Neg | |
| INJICERT-681------InjiCertify_GetCredentialForMockIDA_IdpAccessToken_MoreThanOne_Format_Neg | |
| #MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_Pos | |
| INJICERT-681------InjiCertify_GetCredentialForMockIDA_IdpAccessToken_Invalid_Format_Neg | |
| INJICERT-681------InjiCertify_GetCredentialForMockIDA_IdpAccessToken_MoreThanOne_Format_Neg |
🤖 Prompt for AI Agents
In api-test/src/main/resources/testCaseSkippedList.txt around lines 2 to 4, line
2 currently concatenates two test IDs
("#MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_PosINJICERT-681"); edit
that line to remove the trailing "INJICERT-681" so it reads only
"#MOSIP-12456------Mimoto_AddIdentity_Binding_smoke_Pos", ensuring each test ID
is on its own line and the existing INJICERT-681 entry remains as the separate
next line.
Comment on lines
+5
to
6
| mosip_certify: | ||
| enabled: true |
There was a problem hiding this comment.
🧩 Analysis chain
Rename and branch bump look consistent; verify downstream consumers.
Confirm any scripts/templates that referenced databases.inji_certify now use databases.mosip_certify, and that tag/branch v0.10.2 exists for DB artifacts.
Also applies to: 15-15
🏁 Script executed:
#!/bin/bash
# Find old/new key usage
rg -n -C2 -P 'databases\.(inji_certify|mosip_certify)'
# Sanity check for unresolved references
rg -n -C2 -P '\binji_certify\b' db_scripts deploy helm scripts || trueLength of output: 6874
Update remaining inji_certify references to mosip_certify and verify branch/tag v0.10.2
- Replace
databases.inji_certifywithdatabases.mosip_certifyin db_scripts/README.md (line 26). - Update all hard-coded DB names in db_scripts/mosip_certify/*.sql and deploy.properties (
inji_certify→mosip_certify). - Amend init_db.sh (lines 23–24) to echo and helm delete
mosip_certifyinstead ofinji_certify. - Ensure the
v0.10.2branch or tag exists for the DB artifacts.
🤖 Prompt for AI Agents
In db_scripts/init_values.yaml around lines 5-6, remaining references to the old
`inji_certify` name need to be updated to `mosip_certify` and DB artifacts must
target the v0.10.2 branch/tag; update db_scripts/README.md (around line 26) to
replace `databases.inji_certify` with `databases.mosip_certify`,
search-and-replace all hard-coded DB names `inji_certify` → `mosip_certify` in
db_scripts/mosip_certify/*.sql and in deploy.properties, modify init_db.sh
(lines 23–24) to echo and run `helm delete mosip_certify` instead of
`inji_certify`, and confirm that a branch or tag named v0.10.2 exists for the DB
artifacts (create or point to it if missing).
| @@ -0,0 +1 @@ | |||
| \echo 'Upgrade Queries not required for transition from $CURRENT_VERSION to $UPGRADE_VERSION' No newline at end of file | |||
There was a problem hiding this comment.
psql var expansion uses :VAR, not $VAR.
As written, $CURRENT_VERSION/$UPGRADE_VERSION will print literally. Use psql vars or drop them.
-\echo 'Upgrade Queries not required for transition from $CURRENT_VERSION to $UPGRADE_VERSION'
+\echo 'Upgrade queries not required for transition from :CURRENT_VERSION to :UPGRADE_VERSION'
+-- (ensure CURRENT_VERSION and UPGRADE_VERSION are set via \set or -v, or remove vars:)
+-- \echo 'Upgrade queries not required for this transition'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| \echo 'Upgrade Queries not required for transition from $CURRENT_VERSION to $UPGRADE_VERSION' | |
| \echo 'Upgrade queries not required for transition from :CURRENT_VERSION to :UPGRADE_VERSION' | |
| -- (ensure CURRENT_VERSION and UPGRADE_VERSION are set via \set or -v, or remove vars:) | |
| -- \echo 'Upgrade queries not required for this transition' |
🤖 Prompt for AI Agents
In db_upgrade_script/mosip_certify/sql/0.10.0_to_0.10.1_upgrade.sql around line
1, the psql variable expansion uses $CURRENT_VERSION and $UPGRADE_VERSION which
will not be expanded; replace them with psql-style variables (:CURRENT_VERSION
and :UPGRADE_VERSION) or remove the variables and hardcode/omit them. Update the
\echo line to use :CURRENT_VERSION and :UPGRADE_VERSION (or drop the variables
entirely) so psql prints the intended values.
| @@ -0,0 +1 @@ | |||
| \echo 'Upgrade Queries not required for transition from $CURRENT_VERSION to $UPGRADE_VERSION' No newline at end of file | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Same variable expansion issue as rollback script.
Use psql :vars (or a portable SQL echo) so the message reflects actual versions.
-\echo 'Upgrade Queries not required for transition from $CURRENT_VERSION to $UPGRADE_VERSION'
+\echo Upgrade Queries not required for transition from :CURRENT_VERSION to :UPGRADE_VERSION🏁 Script executed:
#!/bin/bash
find db_upgrade_script/mosip_certify/sql -maxdepth 1 -type fLength of output: 517
🏁 Script executed:
sed -n '1,10p' db_upgrade_script/mosip_certify/sql/0.10.1_to_0.10.2_rollback.sqlLength of output: 94
🏁 Script executed:
#!/bin/bash
sed -n '1,30p' db_upgrade_script/mosip_certify/sql/0.10.1_to_0.10.2_upgrade.sqlLength of output: 93
🏁 Script executed:
grep -R "\\\\set CURRENT_VERSION" -n db_upgrade_script/mosip_certify/sqlLength of output: 0
Mirror rollback fix: use psql variable syntax in upgrade message
As in 0.10.1_to_0.10.2_rollback.sql, change the echo to use :CURRENT_VERSION/ :UPGRADE_VERSION for proper expansion.
-\echo 'Upgrade Queries not required for transition from $CURRENT_VERSION to $UPGRADE_VERSION'
+\echo Upgrade Queries not required for transition from :CURRENT_VERSION to :UPGRADE_VERSION📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| \echo 'Upgrade Queries not required for transition from $CURRENT_VERSION to $UPGRADE_VERSION' | |
| \echo Upgrade Queries not required for transition from :CURRENT_VERSION to :UPGRADE_VERSION |
🤖 Prompt for AI Agents
In db_upgrade_script/mosip_certify/sql/0.10.1_to_0.10.2_upgrade.sql around line
1, the echo uses shell-style $CURRENT_VERSION/$UPGRADE_VERSION which won't
expand as psql variables; update the message to use psql variable syntax
:CURRENT_VERSION and :UPGRADE_VERSION (e.g. replace $CURRENT_VERSION with
:CURRENT_VERSION and $UPGRADE_VERSION with :UPGRADE_VERSION) so the versions are
expanded correctly during execution.
Comment on lines
+354
to
+516
| Example vd12: | ||
| credential_issuer: 'http://localhost:8090' | ||
| credential_endpoint: 'http://localhost:8090/v1/certify/issuance/credential' | ||
| credential_endpoint: 'http://localhost:8090/v1/certify/issuance/credential?version=vd12' | ||
| authorization_servers: | ||
| - 'http://localhost:8080' | ||
| display: | ||
| - name: Insurance | ||
| locale: en | ||
| credentials_supported: | ||
| InsuranceCredential: | ||
| format: ldp_vc | ||
| scope: sunbird_rc_insurance_vc_ldp | ||
| cryptographic_binding_methods_supported: | ||
| - 'did:jwk' | ||
| credential_signing_alg_values_supported: | ||
| - Ed25519Signature2020 | ||
| proof_types_supported: | ||
| type: object | ||
| properties: | ||
| proof_signing_alg_values_supported: | ||
| type: array | ||
| items: | ||
| ['RS256'] | ||
| examples: | ||
| - RS256 | ||
| - PS256 | ||
| credential_definition: | ||
| type: | ||
| - VerifiableCredential | ||
| - InsuranceCredential | ||
| credentialSubject: | ||
| fullName: | ||
| display: | ||
| - name: Name | ||
| locale: en | ||
| mobile: | ||
| display: | ||
| - name: Phone Number | ||
| locale: en | ||
| dob: | ||
| display: | ||
| - name: Date of Birth | ||
| locale: en | ||
| gender: | ||
| display: | ||
| - name: Gender | ||
| locale: en | ||
| benefits: | ||
| display: | ||
| - name: Benefits | ||
| locale: en | ||
| email: | ||
| display: | ||
| - name: Email Id | ||
| locale: en | ||
| policyIssuedOn: | ||
| display: | ||
| - name: Policy Issued On | ||
| locale: en | ||
| policyExpiresOn: | ||
| display: | ||
| - name: Policy Expires On | ||
| locale: en | ||
| policyName: | ||
| display: | ||
| - name: Policy Name | ||
| locale: en | ||
| policyNumber: | ||
| display: | ||
| - name: Policy Number | ||
| locale: en | ||
| display: | ||
| - name: Sunbird RC Insurance Verifiable Credential | ||
| locale: en | ||
| logo: | ||
| url: 'https://sunbird.org/images/sunbird-logo-new.png' | ||
| alt_text: a square logo of a Sunbird | ||
| background_color: '#FDFAF9' | ||
| text_color: '#7C4616' | ||
| order: | ||
| - fullName | ||
| - policyName | ||
| - policyExpiresOn | ||
| - policyIssuedOn | ||
| - policyNumber | ||
| - mobile | ||
| - dob | ||
| - gender | ||
| - benefits | ||
| LifeInsuranceCredential: | ||
| format: ldp_vc | ||
| scope: life_insurance_vc_ldp | ||
| cryptographic_binding_methods_supported: | ||
| - 'did:jwk' | ||
| cryptographic_suites_supported: | ||
| - Ed25519Signature2020 | ||
| proof_types_supported: | ||
| - jwt | ||
| credential_definition: | ||
| type: | ||
| - VerifiableCredential | ||
| credentialSubject: | ||
| fullName: | ||
| display: | ||
| - name: Name | ||
| locale: en | ||
| mobile: | ||
| display: | ||
| - name: Phone Number | ||
| locale: en | ||
| dob: | ||
| display: | ||
| - name: Date of Birth | ||
| locale: en | ||
| gender: | ||
| display: | ||
| - name: Gender | ||
| locale: en | ||
| benefits: | ||
| display: | ||
| - name: Benefits | ||
| locale: en | ||
| email: | ||
| display: | ||
| - name: Email Id | ||
| locale: en | ||
| policyIssuedOn: | ||
| display: | ||
| - name: Policy Issued On | ||
| locale: en | ||
| policyExpiresOn: | ||
| display: | ||
| - name: Policy Expires On | ||
| locale: en | ||
| policyName: | ||
| display: | ||
| - name: Policy Name | ||
| locale: en | ||
| policyNumber: | ||
| display: | ||
| - name: Policy Number | ||
| locale: en | ||
| display: | ||
| - name: Life Insurance Verifiable Credential | ||
| locale: en | ||
| logo: | ||
| url: 'https://sunbird.org/images/sunbird-logo-new.png' | ||
| alt_text: a square logo of a Sunbird | ||
| background_color: '#FDFAF9' | ||
| text_color: '#7C4616' | ||
| order: | ||
| - fullName | ||
| - policyName | ||
| - policyExpiresOn | ||
| - policyIssuedOn | ||
| - policyNumber | ||
| - mobile | ||
| - dob | ||
| - gender | ||
| - benefits | ||
| Example vd13: |
There was a problem hiding this comment.
Same issue in “Example vd12”: provide instance examples, not schema.
Use real values (arrays/strings/objects) instead of schema keywords within examples.
🤖 Prompt for AI Agents
In docs/inji-certify-openapi.yaml around lines 354 to 516, the "Example vd12"
block uses schema keywords (type, properties, items, object, etc.) instead of
concrete instance/example values; replace those schema fragments with actual
example values: use real arrays of strings for lists (e.g.
proof_types_supported: ['ldp_vc'] or ['jwt'],
proof_signing_alg_values_supported: ['RS256']), concrete credential_subject
fields with example values (e.g. fullName: "Jane Doe", mobile: "+15551234567",
dob: "1980-01-01", policyNumber: "POL123456"), provide a real
credential_definition.type array and remove nested "properties" objects, and
populate display blocks with literal strings/URLs/colors; ensure all examples
are valid YAML instances (strings, arrays, objects) rather than schema
descriptors.
docs/inji-certify-openapi.yaml
Outdated
Comment on lines
1131
to
+1496
| type: object | ||
| properties: | ||
| format: | ||
| type: string | ||
| proof: | ||
| $ref: '#/components/schemas/CredentialProof' | ||
| credential_definition: | ||
| $ref: '#/components/schemas/CredentialDefinition' | ||
| oneOf: | ||
| - title: CredentialRequest (mso_modc) | ||
| required: | ||
| - format | ||
| - proof | ||
| - doctype | ||
| - claims | ||
| properties: | ||
| format: | ||
| enum: [mso_mdoc] | ||
| doctype: | ||
| type: string | ||
| minLength: 1 | ||
| example: org.iso.18013.5.1.mDL | ||
| claims: | ||
| type: object | ||
| additionalProperties: | ||
| type: object | ||
| additionalProperties: true | ||
| minProperties: 1 | ||
| proof: | ||
| $ref: '#/components/schemas/CredentialProof' | ||
| credential_definition: | ||
| not: {} | ||
| - title: CredentialRequest (other formats) | ||
| required: | ||
| - format | ||
| - proof | ||
| - credential_definition | ||
| properties: | ||
| format: | ||
| enum: [jwt_vc_json, jwt_vc_json-ld, ldp_vc] | ||
| credential_definition: | ||
| $ref: '#/components/schemas/CredentialDefinition' | ||
| proof: | ||
| $ref: '#/components/schemas/CredentialProof' | ||
|
|
||
|
|
There was a problem hiding this comment.
Typo in title and minor polish in oneOf branch names.
Use “mso_mdoc” consistently; optional: add descriptions for both oneOf branches.
- - title: CredentialRequest (mso_modc)
+ - title: CredentialRequest (mso_mdoc)
required:
- format
- proof
- doctype
- claimsOptionally add description fields to each oneOf branch for clarity.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| type: object | |
| properties: | |
| format: | |
| type: string | |
| proof: | |
| $ref: '#/components/schemas/CredentialProof' | |
| credential_definition: | |
| $ref: '#/components/schemas/CredentialDefinition' | |
| oneOf: | |
| - title: CredentialRequest (mso_modc) | |
| required: | |
| - format | |
| - proof | |
| - doctype | |
| - claims | |
| properties: | |
| format: | |
| enum: [mso_mdoc] | |
| doctype: | |
| type: string | |
| minLength: 1 | |
| example: org.iso.18013.5.1.mDL | |
| claims: | |
| type: object | |
| additionalProperties: | |
| type: object | |
| additionalProperties: true | |
| minProperties: 1 | |
| proof: | |
| $ref: '#/components/schemas/CredentialProof' | |
| credential_definition: | |
| not: {} | |
| - title: CredentialRequest (other formats) | |
| required: | |
| - format | |
| - proof | |
| - credential_definition | |
| properties: | |
| format: | |
| enum: [jwt_vc_json, jwt_vc_json-ld, ldp_vc] | |
| credential_definition: | |
| $ref: '#/components/schemas/CredentialDefinition' | |
| proof: | |
| $ref: '#/components/schemas/CredentialProof' | |
| type: object | |
| oneOf: | |
| - title: CredentialRequest (mso_mdoc) | |
| required: | |
| - format | |
| - proof | |
| - doctype | |
| - claims | |
| properties: | |
| format: | |
| enum: [mso_mdoc] | |
| doctype: | |
| type: string | |
| minLength: 1 | |
| example: org.iso.18013.5.1.mDL | |
| claims: | |
| type: object | |
| additionalProperties: | |
| type: object | |
| additionalProperties: true | |
| minProperties: 1 | |
| proof: | |
| $ref: '#/components/schemas/CredentialProof' | |
| credential_definition: | |
| not: {} | |
| - title: CredentialRequest (other formats) | |
| required: | |
| - format | |
| - proof | |
| - credential_definition | |
| properties: | |
| format: | |
| enum: [jwt_vc_json, jwt_vc_json-ld, ldp_vc] | |
| credential_definition: | |
| $ref: '#/components/schemas/CredentialDefinition' | |
| proof: | |
| $ref: '#/components/schemas/CredentialProof' |
🤖 Prompt for AI Agents
In docs/inji-certify-openapi.yaml around lines 1458 to 1496, the first oneOf
branch title has a typo ("mso_modc") and branch names could be clearer; update
the title to "CredentialRequest (mso_mdoc)" (matching the enum value
"mso_mdoc"), ensure any other occurrences use "mso_mdoc" consistently, and add a
concise description field to each oneOf branch (both the mso_mdoc branch and the
other-formats branch) explaining the intended format and required fields for
readability.
|
|
||
| This document explains the integration of Credential rendering templates in Inji Certify using [VC Render Specification](https://w3c-ccg.github.io/vc-render-method/). | ||
|
|
||
| 1. To use the Verifiable Credential Data Model 2.0 optional features one can configure them in the Velocity Template present in [this file](./certify_init.sql)as per [this draft spec](https://w3c-ccg.github.io/vc-render-method/). The Render Template has to be routable by all the clients and should be cached appropriately. The template is not expected to be updated as the consuming clients are expected to verify the integrity with the provided `digestMultibase`. For detailed information please go through the draft spec. |
There was a problem hiding this comment.
🧩 Analysis chain
Fix broken/ambiguous link and wording.
The link to ./certify_init.sql likely isn’t in this docs folder and will 404; also the sentence is hard to parse.
Proposed edit:
-1. To use the Verifiable Credential Data Model 2.0 optional features one can configure them in the Velocity Template present in [this file](./certify_init.sql)as per [this draft spec](https://w3c-ccg.github.io/vc-render-method/).
+1. To use optional Verifiable Credential Data Model 2.0 features, configure them in the Velocity template (see the DB init SQL in the repo; correct path TBC) as per the [VC Render Method draft](https://w3c-ccg.github.io/vc-render-method/).Please confirm the correct path to the SQL that seeds rendering templates and update the link.
🏁 Script executed:
#!/bin/bash
# Find the location(s) of certify_init.sql
fd -HI certify_init.sqlLength of output: 56
Fix link to certify_init.sql and clarify wording
-1. To use the Verifiable Credential Data Model 2.0 optional features one can configure them in the Velocity Template present in [this file](./certify_init.sql)as per [this draft spec](https://w3c-ccg.github.io/vc-render-method/).
+1. To use optional Verifiable Credential Data Model 2.0 features, configure them in the Velocity template defined in the database initialization SQL (see the [certify_init.sql](../docker-compose/docker-compose-injistack/certify_init.sql)) as per the [VC Render Method draft spec](https://w3c-ccg.github.io/vc-render-method/).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. To use the Verifiable Credential Data Model 2.0 optional features one can configure them in the Velocity Template present in [this file](./certify_init.sql)as per [this draft spec](https://w3c-ccg.github.io/vc-render-method/). The Render Template has to be routable by all the clients and should be cached appropriately. The template is not expected to be updated as the consuming clients are expected to verify the integrity with the provided `digestMultibase`. For detailed information please go through the draft spec. | |
| 1. To use optional Verifiable Credential Data Model 2.0 features, configure them in the Velocity template defined in the database initialization SQL (see the [certify_init.sql](../docker-compose/docker-compose-injistack/certify_init.sql)) as per the [VC Render Method draft spec](https://w3c-ccg.github.io/vc-render-method/). The Render Template has to be routable by all the clients and should be cached appropriately. The template is not expected to be updated as the consuming clients are expected to verify the integrity with the provided `digestMultibase`. For detailed information please go through the draft spec. |
Comment on lines
+31
to
+37
| Note over User,TemplateStore: ==================== Setup Phase ==================== | ||
| Admin->>TemplateStore: Add New Template using sql queris | ||
| TemplateStore-->>Admin: Template ID | ||
| Note over Config: SVG Rendering Template Template Configuration | ||
| Admin->>Config: Configure mosip.certify.data-provider-plugin.rendering-template-id | ||
| Config->>RenderingService: Load template mappings |
There was a problem hiding this comment.
Correct typos in the sequence diagram notes.
“sql queris” → “SQL queries”; “Template Template” → “Template”.
- Admin->>TemplateStore: Add New Template using sql queris
+ Admin->>TemplateStore: Add new template using SQL queries
- Note over Config: SVG Rendering Template Template Configuration
+ Note over Config: SVG Rendering Template Configuration
``` <!-- review_comment_end -->
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
Note over User,TemplateStore: ==================== Setup Phase ====================
Admin->>TemplateStore: Add new template using SQL queries
TemplateStore-->>Admin: Template ID
Note over Config: SVG Rendering Template Configuration
Admin->>Config: Configure mosip.certify.data-provider-plugin.rendering-template-id
Config->>RenderingService: Load template mappings🤖 Prompt for AI Agents
In docs/Rendering-Template.md around lines 31 to 37, fix typos in the sequence
diagram notes by replacing "sql queris" with "SQL queries" and removing the
duplicated word in "SVG Rendering Template Template Configuration" so it reads
"SVG Rendering Template Configuration"; update the diagram text accordingly to
preserve capitalization and spacing.
Contributor
Author
There was a problem hiding this comment.
@CodeRabbit what kind of loops are we using here most ?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Searching for loop patterns in Java files ==="
echo ""
echo "1. Traditional for loops:"
rg -t java '^\s*for\s*\(' --count-matches | awk -F: '{sum+=$2} END {print "Total: " sum}'
echo ""
echo "2. Enhanced for loops (for-each):"
rg -t java '^\s*for\s*\([^;]+:\s*' --count-matches | awk -F: '{sum+=$2} END {print "Total: " sum}'
echo ""
echo "3. While loops:"
rg -t java '^\s*while\s*\(' --count-matches | awk -F: '{sum+=$2} END {print "Total: " sum}'
echo ""
echo "4. Do-while loops:"
rg -t java '^\s*do\s*\{' --count-matches | awk -F: '{sum+=$2} END {print "Total: " sum}'
echo ""
echo "5. forEach method calls:"
rg -t java '\.forEach\s*\(' --count-matches | awk -F: '{sum+=$2} END {print "Total: " sum}'
echo ""
echo "6. Stream operations (map, filter, etc.):"
rg -t java '\.(map|filter|flatMap|collect)\s*\(' --count-matches | awk -F: '{sum+=$2} END {print "Total: " sum}'
echo ""
echo "=== Top 10 files with most loops ==="
rg -t java '^\s*(for|while)\s*\(' -c | sort -t: -k2 -nr | head -10
Length of output: 1179
@Mahesh-Binayak Based on analyzing the codebase, here's the breakdown of loop usage:
Loop Usage Statistics
-
Traditional
forloops: 27 occurrences (most common)- Example:
for (int i = 0; i < size; i++)
- Example:
-
Enhanced for-each loops: 18 occurrences (second most common)
- Example:
for (String item : collection)
- Example:
-
Stream operations (map, filter, flatMap, collect): 3 occurrences
-
While loops: 2 occurrences
-
forEach method calls: 1 occurrence
-
Do-while loops: 0 occurrences
Key Findings
- Traditional
forloops are used most frequently in this codebase, accounting for more than half of all loop constructs - The codebase shows a preference for imperative-style loops over functional programming patterns
- The files with the most loops are:
VCIssuanceServiceImpl.java(5 loops)CertifyIssuanceServiceImpl.java(5 loops)MosipTestRunner.java(3 loops)
The predominant use of traditional and enhanced for loops suggests the codebase follows a more classical Java programming style rather than modern functional/stream-based approaches.
Contributor
Author
There was a problem hiding this comment.
@CodeRabbit how do I optimize the MosipTestRunner.java code further ?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display the MosipTestRunner.java file
echo "=== Locating MosipTestRunner.java ==="
fd -t f "MosipTestRunner.java"
echo ""
echo "=== Reading MosipTestRunner.java content ==="
fd -t f "MosipTestRunner.java" --exec cat {}
Length of output: 11862
@Mahesh-Binayak Here are specific optimization recommendations for MosipTestRunner.java:
🚀 High-Priority Optimizations
1. Break Down the Monolithic main() Method
The main method has too many responsibilities. Extract into smaller methods:
public static void main(String[] arg) {
try {
logEnvironmentVariables();
initializeTestFramework();
setupHealthChecker();
prepareTestEnvironment();
startTestRunner();
} catch (Exception e) {
LOGGER.error("Test execution failed", e);
} finally {
cleanup();
System.exit(0);
}
}2. Use Try-With-Resources for FileInputStream
Replace manual resource management in getproperty():
public static Properties getproperty(String path) {
Properties prop = new Properties();
try (FileInputStream inputStream = new FileInputStream(path)) {
prop.load(inputStream);
} catch (IOException e) {
LOGGER.error("Failed to load properties from: " + path, e);
}
return prop;
}3. Optimize Loop in startTestRunner()
Current nested loops can be optimized:
// Current: nested loops with repeated checks
for (String useCase : useCases) {
for (File file : files) {
if (file.getName().toLowerCase().contains("mastertestsuite")) {
// ... setup and run
}
}
}
// Optimized: filter files once, then iterate use cases
List<File> masterTestSuites = Arrays.stream(files)
.filter(f -> f.getName().toLowerCase().contains("mastertestsuite"))
.collect(Collectors.toList());
for (String useCase : useCases) {
for (File file : masterTestSuites) {
runTestSuite(useCase, file);
}
}4. Cache KeyPairGenerator Properly with Thread-Safety
private static final class KeyPairGeneratorHolder {
private static final KeyPairGenerator INSTANCE = initKeyPairGenerator();
private static KeyPairGenerator initKeyPairGenerator() {
try {
KeyPairGenerator gen = KeyPairGenerator.getInstance("RSA");
gen.initialize(2048);
return gen;
} catch (NoSuchAlgorithmException e) {
throw new IllegalStateException("RSA algorithm not available", e);
}
}
}
public static KeyPairGenerator getKeyPairGeneratorInstance() {
return KeyPairGeneratorHolder.INSTANCE;
}5. Replace String Concatenation with StringBuilder in Loops
In main() method's environment variable logging:
StringBuilder envLog = new StringBuilder("** Get ALL ENV variables **\n");
for (Map.Entry<String, String> entry : envMap.entrySet()) {
envLog.append("ENV ").append(entry.getKey())
.append(" = ").append(entry.getValue()).append("\n");
}
LOGGER.info(envLog.toString());6. Use Modern Java Path API
// Replace File operations with Path API
public static String getGlobalResourcePath() {
if (cachedPath != null) {
return cachedPath;
}
Path path;
if (getRunType().equalsIgnoreCase("JAR")) {
path = Paths.get(jarUrl).getParent()
.resolve("MosipTestResource/MosipTemporaryTestResource");
} else {
path = Paths.get(MosipTestRunner.class.getClassLoader()
.getResource("").toURI())
.resolve("MosipTestResource/MosipTemporaryTestResource");
}
cachedPath = path.toString();
return cachedPath;
}🎯 Medium-Priority Optimizations
- Extract Magic Strings to Constants
private static final String MASTER_TEST_SUITE = "mastertestsuite";
private static final String RSA_ALGORITHM = "RSA";
private static final int RSA_KEY_SIZE = 2048;-
Reduce Code Duplication in Key Generation Methods
Create a common method for KeyPairGenerator initialization used acrossgeneratePulicKey(),generatePublicKeyForMimoto(), andgenerateJWKPublicKey(). -
Use Diamond Operator and Type Inference
// Replace
List<String> languageList = new ArrayList<>();
// With
List<String> languageList = new ArrayList<>(); // Already optimized- Replace Log4j with SLF4J (if possible)
private static final Logger LOGGER = LoggerFactory.getLogger(MosipTestRunner.class);📊 Performance Impact
- Breaking down main(): Improves maintainability and testability
- Try-with-resources: Prevents resource leaks
- Loop optimization: Reduces file iteration from O(n×m) to O(n+m)
- StringBuilder: ~30-50% faster for string concatenation in loops
- Path API caching: Thread-safe and more efficient
Would you like me to create a refactored version of any specific section?
Contributor
Author
There was a problem hiding this comment.
@CodeRabbit , are all licenses used in the code changes, compliant with copy-right license authorities like MPL and Apache or is there some restrictive license in use (even transitively) ?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Finding all POM files in the repository ==="
fd -t f "pom.xml"
echo ""
echo "=== Extracting direct dependencies from root pom.xml ==="
if [ -f "pom.xml" ]; then
echo "Root POM dependencies:"
cat pom.xml | grep -A 3 "<dependency>" | head -50
fi
echo ""
echo "=== Checking for license declarations in POMs ==="
rg -t xml "<license>" -A 5 pom.xml
echo ""
echo "=== Looking for specific dependency versions mentioned in PR ==="
echo "Checking kernel.keymanager.version:"
rg "kernel\.keymanager\.version" pom.xml -A 1 -B 1
echo ""
echo "Checking apitest-commons:"
rg "apitest-commons" -A 3 -B 1
echo ""
echo "=== Checking if Maven is available and trying to list dependencies ==="
if command -v mvn &> /dev/null; then
echo "Maven found, attempting to list dependencies with licenses..."
mvn dependency:list -DoutputFile=/tmp/deps.txt 2>&1 | head -20
if [ -f /tmp/deps.txt ]; then
cat /tmp/deps.txt | head -50
fi
else
echo "Maven not available in sandbox environment"
fi
echo ""
echo "=== Searching for LICENSE files in the repository ==="
fd -t f -i "license" --max-depth 2
echo ""
echo "=== Checking common restrictive license indicators ==="
rg -i "(GPL|AGPL|LGPL|SSPL|Commons Clause)" --type-add 'license:LICENSE*' -t license -g "LICENSE*" -g "*.md" | head -20
Length of output: 2699
🏁 Script executed:
#!/bin/bash
echo "=== Reading LICENSE file ==="
cat LICENSE
echo ""
echo "=== Checking all POM files for GPL/AGPL/LGPL dependencies ==="
rg -i "(gpl|agpl|lgpl)" pom.xml api-test/pom.xml certify-core/pom.xml certify-integration-api/pom.xml certify-service/pom.xml
echo ""
echo "=== Checking dependency groupIds for common permissive license sources ==="
echo "Spring Framework (Apache 2.0):"
rg "org\.springframework" pom.xml -c
echo ""
echo "Apache Commons (Apache 2.0):"
rg "org\.apache\.commons" pom.xml certify-*/pom.xml api-test/pom.xml
echo ""
echo "=== Looking for MOSIP internal dependencies ==="
rg "io\.mosip" pom.xml certify-*/pom.xml api-test/pom.xml | head -20
echo ""
echo "=== Checking for any exclusions or specific license notes ==="
rg -i "license|copyright" pom.xml -B 2 -A 2
echo ""
echo "=== Looking at major third-party dependencies ==="
echo "jose4j (Apache 2.0):"
rg "jose4j" pom.xml -B 1 -A 2
echo ""
echo "Lombok (MIT):"
rg "lombok" pom.xml -B 1 -A 2
echo ""
echo "Jackson (Apache 2.0):"
rg "jackson" pom.xml api-test/pom.xml certify-*/pom.xml -c
Length of output: 19209
@Mahesh-Binayak Based on my analysis of the code changes and dependencies, here's the license compliance assessment:
✅ License Compliance Summary
All identified licenses are permissive and compliant with MPL 2.0 and Apache 2.0 standards. No restrictive licenses detected.
📋 License Breakdown
Primary Project License:
- MPL 2.0 (Mozilla Public License 2.0) - The project's own license
Direct Dependencies:
- Spring Framework - Apache 2.0 ✅
- jose4j (0.6.5) - Apache 2.0 ✅
- Lombok (1.18.30) - MIT License ✅
- Jackson - Apache 2.0 ✅
- MOSIP internal libraries (
io.mosip.*) - MPL 2.0 ✅
🔍 Key Findings
- ✅ No GPL/AGPL/LGPL dependencies found in any POM files
- ✅ All licenses are permissive (MPL 2.0, Apache 2.0, MIT)
- ✅ MPL 2.0 is explicitly compatible with Apache 2.0 and MIT licenses
- ✅ MPL 2.0 Section 3.3 allows combination with Secondary Licenses, providing additional flexibility
🔄 Version Updates in This PR
The dependency version changes don't introduce any new restrictive licenses:
kernel.keymanager.version: 1.3.0-SNAPSHOT → 1.3.0-beta.2 (MOSIP/MPL 2.0)apitest-commons: 1.3.1-SNAPSHOT → 1.3.1 (MOSIP/MPL 2.0)
⚠️ Important Note
Transitive Dependencies: While I've verified all direct dependencies, a complete transitive dependency analysis requires running:
mvn dependency:tree
mvn license:aggregate-third-party-reportThis will generate a comprehensive report of all transitive dependencies and their licenses. However, based on the ecosystem (Spring Boot, MOSIP libraries), transitive dependencies are expected to be similarly permissive (mostly Apache 2.0).
📚 License Compatibility
MPL 2.0 is specifically designed for compatibility:
- ✅ Compatible with Apache 2.0
- ✅ Compatible with MIT
- ✅ Compatible with BSD licenses
- ✅ Allows linking with proprietary code
- ✅ Section 3.3 explicitly permits combination with GPL/LGPL as "Secondary Licenses"
Conclusion: The codebase demonstrates good license hygiene with no restrictive licenses present in the analyzed dependencies.
Prafulrakhade
force-pushed
the
master
branch
from
47a4625 to
e1f3509
Compare
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 by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores