Skip to content

Closes #1828 Volume mounting is not working if git local repo is selcted#1838

Merged
ckm007 merged 2 commits into
mosip:developfrom
abhishek-1809:develop
Jun 16, 2026
Merged

Closes #1828 Volume mounting is not working if git local repo is selcted#1838
ckm007 merged 2 commits into
mosip:developfrom
abhishek-1809:develop

Conversation

@abhishek-1809

@abhishek-1809 abhishek-1809 commented Jun 15, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

Chores

  • Enhanced installation configuration to allow users to select their preferred configuration source (remote Git, local NFS, or both)
  • Updated persistent storage class to nfs-csi for improved compatibility
  • Streamlined repository credential configuration in deployment manifests

… selcted

Signed-off-by: abhishek-1809 <abhisahu1920@gmail.com>
… selcted

Signed-off-by: abhishek-1809 <abhisahu1920@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The changes migrate config-server from manual NFS PV wiring to CSI-based storage (nfs-csi), add composite Spring profiles support, rename composite repo secret keys to SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_<index>_PASSWORD, remove the standalone PV template, simplify PVC and volume mount guards to localRepo.enabled, and replace the install script's individual prompts with a menu-driven configuration source selection.

Changes

config-server composite profile and CSI storage migration

Layer / File(s) Summary
Values: composite profile and nfs-csi storage defaults
helm/config-server/values.yaml, deploy/config-server/values.yaml
Both values files set spring_profiles_active to "composite", switch storageClass from nfs-client to nfs-csi, and remove the inline nfs path/server configuration.
PV removal and PVC guard simplification
helm/config-server/templates/config-pv.yaml, helm/config-server/templates/config-pvc.yaml
The manual PersistentVolume template is deleted entirely; the PVC template's inclusion guard is reduced to .Values.localRepo.enabled only, and spec.selector.matchLabels is removed.
Deployment env var, volume wiring, and secret key rename
helm/config-server/templates/deployment.yaml, helm/config-server/templates/secret.yaml
SPRING_PROFILES_ACTIVE gains a three-way conditional that concatenates both profile sources when both are active. The composite repo password secret key is renamed from github-token-<index> to SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_<index>_PASSWORD (with base64 encoding in the Secret). Volume mounts and volumes are now gated solely on localRepo.enabled.
Install script: menu-driven config source selection
deploy/config-server/install.sh
NFS path/server prompts and the prior local-repo yes/no prompt are replaced with a numbered menu (remote Git / local NFS / both) that derives SPRING_PROFILES and LOCALREPO. The helm install invocation gains --set spring_profiles.enabled and drops volume.nfs.path/volume.nfs.server.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hop, hop, no more NFS server to type,
A menu now picks what config feels right—
Composite profiles blend local and remote,
The PV template vanished without a note.
nfs-csi takes the helm, keys are renamed bright,
The bunny refactors by CSI moonlight! 🌙

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately references the issue being fixed (#1828) and describes the core problem being resolved: volume mounting not working when git local repo is selected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
helm/config-server/templates/secret.yaml (1)

12-15: ⚡ Quick win

Apply base64 encoding consistently, or remove the else branch.

Line 12 correctly uses b64enc for the token, but line 14 omits it for the empty case. For semantic consistency with Kubernetes Secret data encoding, apply b64enc to the empty string as well.

Better yet, consider removing the else branch entirely (lines 13-15), since the deployment template (lines 86-92 in deployment.yaml) only creates the environment variable when $repo.token exists. An empty secret key is never referenced and adds clutter.

Recommended refactor

Option 1 (preferred): Remove the else branch

  {{- range $index, $repo := .Values.spring_profiles.spring_compositeRepos }}
  {{- if $repo.token }}
  SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_{{ $index }}_PASSWORD: {{ $repo.token | b64enc | quote }}
- {{- else }}
- SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_{{ $index }}_PASSWORD: ""
  {{- end }}
  {{- end }}

Option 2: Apply b64enc consistently

  {{- range $index, $repo := .Values.spring_profiles.spring_compositeRepos }}
  {{- if $repo.token }}
  SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_{{ $index }}_PASSWORD: {{ $repo.token | b64enc | quote }}
  {{- else }}
- SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_{{ $index }}_PASSWORD: ""
+ SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_{{ $index }}_PASSWORD: {{ "" | b64enc | quote }}
  {{- end }}
  {{- end }}
🤖 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 `@helm/config-server/templates/secret.yaml` around lines 12 - 15, The
SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_{{ $index }}_PASSWORD environment variable
definition has an inconsistent else branch. Either remove the entire else branch
(lines 13-15) since the deployment template only references this secret key when
$repo.token exists and an empty string is never used, or if you prefer to keep
the else branch, apply the b64enc filter consistently to the empty string on
line 14 to match the encoding applied to the token on line 12.
🤖 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 `@helm/config-server/templates/secret.yaml`:
- Around line 12-15: The SPRING_CLOUD_CONFIG_SERVER_COMPOSITE_{{ $index
}}_PASSWORD environment variable definition has an inconsistent else branch.
Either remove the entire else branch (lines 13-15) since the deployment template
only references this secret key when $repo.token exists and an empty string is
never used, or if you prefer to keep the else branch, apply the b64enc filter
consistently to the empty string on line 14 to match the encoding applied to the
token on line 12.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad95d6eb-cd55-4bcc-970a-bc7df539c329

📥 Commits

Reviewing files that changed from the base of the PR and between eb577a3 and e6940df.

📒 Files selected for processing (7)
  • deploy/config-server/install.sh
  • deploy/config-server/values.yaml
  • helm/config-server/templates/config-pv.yaml
  • helm/config-server/templates/config-pvc.yaml
  • helm/config-server/templates/deployment.yaml
  • helm/config-server/templates/secret.yaml
  • helm/config-server/values.yaml
💤 Files with no reviewable changes (1)
  • helm/config-server/templates/config-pv.yaml

@ckm007 ckm007 merged commit 387d218 into mosip:develop Jun 16, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants