[MOSIP-44200] updated helm charts changes to publish to new inji helm#503
[MOSIP-44200] updated helm charts changes to publish to new inji helm#503
Conversation
Signed-off-by: Chandra Keshav Mishra <chandrakeshavmishra@gmail.com>
WalkthroughReplaced mosip Helm repository references with inji across CI workflow, install script, and Helm values; updated image repositories, labels, and container user values; removed a redundant host variable assignment in the install script. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deploy/inji-web/install.sh (2)
19-22: Critical bug: Missing$before variable name.The condition
[ -z "INJIWEB_HOST" ]checks if the literal string"INJIWEB_HOST"is empty (which is always false), not the variable's value. This means the script will never exit even whenINJIWEB_HOSTis empty.🐛 Proposed fix
- if [ -z "INJIWEB_HOST" ]; then + if [ -z "$INJIWEB_HOST" ]; then echo "INJIWEB Host not provided; EXITING;" exit 0; fi
31-38: Logic error: Configmap patch always executes regardless of condition.Lines 31-38 execute unconditionally after the if-else block (lines 14-23), meaning the configmap will be patched even when
INJIWEB_HOSTis already present. The echo message "INJIWEB_HOST is not present..." is misleading since this code runs in both cases.This block should be inside the
elsebranch (lines 16-23).🐛 Proposed fix
if echo "$DEFAULT_INJIWEB_HOST" | grep -q "INJIWEB_HOST"; then echo "INJIWEB_HOST is already present in configmap/inji-stack-config of configserver" else read -p "Please provide injiwebhost (eg: injiweb.sandbox.xyz.net ) : " INJIWEB_HOST - if [ -z "INJIWEB_HOST" ]; then + if [ -z "$INJIWEB_HOST" ]; then echo "INJIWEB Host not provided; EXITING;" exit 0; fi -fi - -CHK_INJIWEB_HOST=$( nslookup "$INJIWEB_HOST" ) -if [ $? -gt 0 ]; then - echo "Injiweb Host does not exists; EXITING;" - exit 0; -fi -echo "INJIWEB_HOST is not present in configmap/inji-stack-config of configserver" + CHK_INJIWEB_HOST=$( nslookup "$INJIWEB_HOST" ) + if [ $? -gt 0 ]; then + echo "Injiweb Host does not exist; EXITING;" + exit 0; + fi + + echo "INJIWEB_HOST is not present in configmap/inji-stack-config of configserver" # Add injiweb host to inji-stack-config kubectl patch configmap inji-stack-config -n config-server --type merge -p "{\"data\": {\"injiweb-host\": \"$INJIWEB_HOST\"}}" kubectl patch configmap inji-stack-config -n default --type merge -p "{\"data\": {\"injiweb-host\": \"$INJIWEB_HOST\"}}" # Add the host kubectl -n config-server set env --keys=injiweb-host --from configmap/inji-stack-config deployment/config-server --prefix=SPRING_CLOUD_CONFIG_SERVER_OVERRIDES_ # Restart the configserver deployment kubectl -n config-server get deploy -o name | xargs -n1 -t kubectl -n config-server rollout status +fi
🤖 Fix all issues with AI agents
In `@deploy/inji-web/install.sh`:
- Line 52: INJI_HOST is assigned but never used and the jsonpath is unquoted;
either remove the INJI_HOST assignment or replace occurrences of INJIWEB_HOST
with INJI_HOST where intended, and ensure the kubectl command uses a quoted
jsonpath (e.g. "-o jsonpath='{.data.injiweb-host}'") to avoid word-splitting;
update the assignment line for INJI_HOST and any references to INJIWEB_HOST
accordingly (look for INJI_HOST and INJIWEB_HOST in the script and the kubectl
get cm inji-stack-config command).
In `@helm/inji-web/values.yaml`:
- Around line 139-140: The comment above the container_user value incorrectly
references "Clamav container" — update or remove that stale reference so it
correctly refers to the inji-web chart (or simply state that the inji-web
container already runs as 'inji' user). Locate the comment near the
container_user key and replace "Clamav" with "inji-web" or reword to avoid any
chart-specific name to prevent future copy-paste confusion.
- Line 14: The values.yaml contains a stale comment mentioning "Clamav
container" and a conflicting security context: remove or correct the stale
comment so it references inji-web (not Clamav), and reconcile the security user
settings by either deleting or changing runAsUser: 0 to match container_user:
'inji' (or explicitly set runAsUser to the UID of 'inji'); update the comment
near container_user and runAsUser to accurately state the intended non-root
security context.
deploy/inji-web/install.sh
Outdated
|
|
||
| ./copy_cm.sh | ||
|
|
||
| INJI_HOST=$(kubectl get cm inji-stack-config -o jsonpath={.data.injiweb-host}) |
There was a problem hiding this comment.
Unused variable INJI_HOST and unquoted command substitution.
INJI_HOST is assigned but never used. Either remove it or use it where INJIWEB_HOST is currently used on lines 55-56. Additionally, the jsonpath argument should be quoted.
📝 Proposed fix (remove unused variable or use it)
Option 1 - Remove unused variable:
-INJI_HOST=$(kubectl get cm inji-stack-config -o jsonpath={.data.injiweb-host})Option 2 - Use the variable (if intended):
-INJI_HOST=$(kubectl get cm inji-stack-config -o jsonpath={.data.injiweb-host})
+INJI_HOST=$(kubectl get cm inji-stack-config -o jsonpath='{.data.injiweb-host}')
echo "Installing INJIWEB"
helm -n $NS install injiweb inji/injiweb \
- --set inji_web.configmaps.injiweb-ui.MIMOTO_URL=https://$INJIWEB_HOST/v1/mimoto \
- --set istio.hosts[0]=$INJIWEB_HOST \
+ --set inji_web.configmaps.injiweb-ui.MIMOTO_URL=https://$INJI_HOST/v1/mimoto \
+ --set istio.hosts[0]=$INJI_HOST \📝 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.
| INJI_HOST=$(kubectl get cm inji-stack-config -o jsonpath={.data.injiweb-host}) |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 52-52: INJI_HOST appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 52-52: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 52-52: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
🤖 Prompt for AI Agents
In `@deploy/inji-web/install.sh` at line 52, INJI_HOST is assigned but never used
and the jsonpath is unquoted; either remove the INJI_HOST assignment or replace
occurrences of INJIWEB_HOST with INJI_HOST where intended, and ensure the
kubectl command uses a quoted jsonpath (e.g. "-o
jsonpath='{.data.injiweb-host}'") to avoid word-splitting; update the assignment
line for INJI_HOST and any references to INJIWEB_HOST accordingly (look for
INJI_HOST and INJIWEB_HOST in the script and the kubectl get cm
inji-stack-config command).
| ## | ||
| commonLabels: | ||
| app.kubernetes.io/component: mosip | ||
| app.kubernetes.io/component: inji |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's view the helm values.yaml file to see the actual changes
cat -n helm/inji-web/values.yaml | head -200Repository: inji/inji-web
Length of output: 7007
🏁 Script executed:
# Continue reading the file to see line 316 and surrounding content
cat -n helm/inji-web/values.yaml | sed -n '300,330p'Repository: inji/inji-web
Length of output: 1401
🏁 Script executed:
# Verify the Docker images exist
echo "=== Checking injistackdev/inji-web image ==="
curl -s "https://hub.docker.com/v2/repositories/injistackdev/inji-web/" | jq -r '.name, .namespace' 2>/dev/null || echo "Image not found or API error"
echo ""
echo "=== Checking mosipid/os-shell image ==="
curl -s "https://hub.docker.com/v2/repositories/mosipid/os-shell/" | jq -r '.name, .namespace' 2>/dev/null || echo "Image not found or API error"Repository: inji/inji-web
Length of output: 181
Address stale comment and security context misconfiguration.
Line 139 references "Clamav container" but this is the inji-web values file—this comment appears to be a copy-paste artifact and should be removed or corrected.
More importantly, line 143 sets runAsUser: 0 (root) which contradicts line 140 (container_user: 'inji') and the stated intent in the comment. Either remove the runAsUser: 0 setting to respect the container_user: 'inji' configuration, or update the configuration and comment to clarify the security context intent.
🤖 Prompt for AI Agents
In `@helm/inji-web/values.yaml` at line 14, The values.yaml contains a stale
comment mentioning "Clamav container" and a conflicting security context: remove
or correct the stale comment so it references inji-web (not Clamav), and
reconcile the security user settings by either deleting or changing runAsUser: 0
to match container_user: 'inji' (or explicitly set runAsUser to the UID of
'inji'); update the comment near container_user and runAsUser to accurately
state the intended non-root security context.
| ## Clamav container already runs as 'inji' user, so we may not need to enable this | ||
| container_user: 'inji' |
There was a problem hiding this comment.
Stale comment references Clamav instead of inji-web.
The comment mentions "Clamav container" but this is the inji-web chart. This appears to be a copy-paste artifact.
📝 Suggested fix
-## Clamav container already runs as 'inji' user, so we may not need to enable this
+## inji-web container already runs as 'inji' user, so we may not need to enable this
container_user: 'inji'🤖 Prompt for AI Agents
In `@helm/inji-web/values.yaml` around lines 139 - 140, The comment above the
container_user value incorrectly references "Clamav container" — update or
remove that stale reference so it correctly refers to the inji-web chart (or
simply state that the inji-web container already runs as 'inji' user). Locate
the comment near the container_user key and replace "Clamav" with "inji-web" or
reword to avoid any chart-specific name to prevent future copy-paste confusion.
Signed-off-by: Abhishek S <127825992+abhishek8shankar@users.noreply.github.com>
|
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.