[MOSIP-44613],[MOSIP-44620],[MOSIP-44622],[MOSIP-44623],[MOSIP-44626],[MOSIP-44627],[MOSIP-44628]Updated profile based deployment with helmsman and terraform structural and workflow changes#173
Merged
bhumi46 merged 21 commits intomosip:developfrom Mar 24, 2026
Conversation
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds profile-aware deployment: workflow inputs and detection, profile-specific Helmsman DSFs and hooks for MOSIP/eSignet, profile tfvars for Terraform, and a Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant WF as Workflow Steps
participant CFG as configure-backend.sh
participant TF as Terraform
participant S3 as Remote Backend (S3/Azure/GCS)
participant HEL as Helmsman
GH->>WF: trigger (push or workflow_dispatch) with/without profile
WF->>WF: detect/assign PROFILE, set GITHUB_ENV / outputs
WF->>CFG: exec configure-backend.sh --profile $PROFILE ...
CFG->>CFG: compute local state filename (include PROFILE if set)
CFG->>S3: generate backend key/prefix (include PROFILE when non-empty)
WF->>TF: terraform plan/apply -var-file="$TFVARS_FILE" (profile-aware)
TF->>S3: read/write remote state using backend key/prefix
WF->>HEL: run Helmsman using DSF at dsf/$PROFILE/...
HEL-->>WF: deployment status
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: bhumi46 <111699703+bhumi46@users.noreply.github.com>
Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/helmsman_external.yml (1)
262-279:⚠️ Potential issue | 🟠 MajorPass the
profileinput to the downstream workflow dispatch call.The workflow-caller condition correctly gates the MOSIP trigger to workflow_dispatch events with mosip-platform-* profiles. However, the
helmsman_mosip.ymlworkflow does accept aprofileinput (required, with defaultmosip-platform-java11), but the API dispatch call only passesmode. This means the profile selection at the trigger point is ignored, and the downstream workflow always uses its default profile.To preserve the profile intent, the curl payload should include the profile input:
{"ref":"<BRANCH>","inputs":{"mode":"apply","profile":"<PROFILE>"}}Alternatively, verify that always using the default profile is the intended behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/helmsman_external.yml around lines 262 - 279, The dispatch payload for the workflow-caller step currently only sends "mode" so the downstream helmsman_mosip.yml never receives the triggering profile; update the curl -d payload in the "Trigger helmsman mosip workflow via API" step to include the profile input (use the workflow's profile value, e.g. github.event.inputs.profile or pass it into an env like PROFILE) alongside BRANCH and mode so the JSON becomes {"ref":"<BRANCH>","inputs":{"mode":"apply","profile":"<PROFILE>"}} and helmsman_mosip.yml will receive the intended profile.
🧹 Nitpick comments (16)
Helmsman/hooks/esignet-1.7.1/redis-setup.sh (2)
14-15: Inconsistent namespace variable handling.
ESIGNET_NSallows environment variable override whileREDIS_NSis hardcoded. For consistency with the pattern used forESIGNET_NS, consider makingREDIS_NSconfigurable.♻️ Proposed fix
ESIGNET_NS="${ESIGNET_NS:-esignet}" -REDIS_NS="redis" +REDIS_NS="${REDIS_NS:-redis}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/redis-setup.sh` around lines 14 - 15, The script inconsistently hardcodes REDIS_NS while ESIGNET_NS supports env overrides; change REDIS_NS to be configurable by defining it with a default value (e.g., set REDIS_NS using the pattern used for ESIGNET_NS) so consumers can override the namespace via environment variable, and update any references to REDIS_NS in this file (and related functions/commands) to rely on that variable.
21-23: Redis wait failure will abort the script.Unlike other hooks in this PR that use
|| echo "WARNING:..."to continue on readiness wait failure, this script will exit on wait timeout due toset -e. If this is intentional (Redis must be ready before proceeding), this is fine. Otherwise, consider adding a warning fallback for consistency.🔧 Optional: Add warning fallback for consistency
# --- Step 1: Wait for Redis to be ready --- echo "Waiting for Redis pods to be ready..." -kubectl -n "$REDIS_NS" wait --for=condition=ready pod -l app.kubernetes.io/name=redis --timeout=300s +kubectl -n "$REDIS_NS" wait --for=condition=ready pod -l app.kubernetes.io/name=redis --timeout=300s || \ + echo "WARNING: Redis pods not ready yet. Continuing with setup."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/redis-setup.sh` around lines 21 - 23, The kubectl readiness wait in redis-setup.sh (the kubectl -n "$REDIS_NS" wait --for=condition=ready pod -l app.kubernetes.io/name=redis --timeout=300s command) will cause the script to abort on timeout because of set -e; to make behavior consistent with other hooks, catch the nonzero exit and log a warning instead of exiting by appending a fallback that prints a clear WARNING (e.g., indicating Redis wait timed out and the script is continuing) so the script continues on readiness failure; update the kubectl wait invocation to handle failure and emit the warning.Helmsman/hooks/esignet-1.7.1/postgres-preinstall.sh (1)
36-47: Consider parameterizing the hardcoded version.The
mosip-version: "1.7.1"is hardcoded. If future versions of this hook are created (e.g.,esignet-1.8.0/), this value would need manual updates. Consider extracting it to an environment variable.♻️ Proposed enhancement
ESIGNET_NS="${ESIGNET_NS:-esignet}" INSTALLATION_DOMAIN="${INSTALLATION_DOMAIN:-sandbox.xyz.net}" +ESIGNET_VERSION="${ESIGNET_VERSION:-1.7.1}" ... data: - mosip-version: "1.7.1" + mosip-version: "${ESIGNET_VERSION}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/postgres-preinstall.sh` around lines 36 - 47, The data YAML in the heredoc currently hardcodes mosip-version: "1.7.1"; change it to read from an environment/template variable (e.g., ${MOSIP_VERSION} or a Helm value) so the hook can be reused for future releases; update the heredoc generation in postgres-preinstall.sh to inject that variable and ensure any callers/exporters set MOSIP_VERSION (or the corresponding Helm value) before invoking the script, and validate fallback behavior if the variable is unset.Helmsman/hooks/esignet-1.7.1/esignet-partner-onboarder-postinstall.sh (1)
13-21: Hardcoded namespace is inconsistent with other hooks in this PR.Other hooks in this PR (e.g.,
kafka-postinstall.sh,redis-setup.sh,softhsm-esignet-postinstall.sh) useESIGNET_NS="${ESIGNET_NS:-esignet}"for namespace configurability. This script hardcodesesignetdirectly.♻️ Proposed fix for consistency
set -euo pipefail +ESIGNET_NS="${ESIGNET_NS:-esignet}" + echo "================================================" echo "eSignet 1.7.1 - Partner Onboarder Post-install" echo "================================================" # Check onboarding job status -JOB_STATUS=$(kubectl -n esignet get jobs -l app.kubernetes.io/instance=esignet-resident-oidc-partner-onboarder -o jsonpath='{.items[0].status.succeeded}' 2>/dev/null || echo "") +JOB_STATUS=$(kubectl -n "$ESIGNET_NS" get jobs -l app.kubernetes.io/instance=esignet-resident-oidc-partner-onboarder -o jsonpath='{.items[0].status.succeeded}' 2>/dev/null || echo "") if [ "$JOB_STATUS" = "1" ]; then echo "Partner onboarding completed successfully." else echo "WARNING: Partner onboarding job may not have completed. Check logs." - kubectl -n esignet logs -l app.kubernetes.io/instance=esignet-resident-oidc-partner-onboarder --tail=20 2>/dev/null || true + kubectl -n "$ESIGNET_NS" logs -l app.kubernetes.io/instance=esignet-resident-oidc-partner-onboarder --tail=20 2>/dev/null || true fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/esignet-partner-onboarder-postinstall.sh` around lines 13 - 21, This script hardcodes the namespace "esignet" causing inconsistency; add the same namespace fallback used elsewhere by defining ESIGNET_NS="${ESIGNET_NS:-esignet}" near the top and replace the hardcoded -n esignet occurrences used by JOB_STATUS assignment and the kubectl logs invocation with -n "$ESIGNET_NS" (ensure the JOB_STATUS variable and both kubectl commands reference the new ESIGNET_NS variable).Helmsman/hooks/esignet-1.7.1/softhsm-esignet-postinstall.sh (1)
30-38: Thesednamespace substitution may be fragile.The pattern
s/namespace: $SOFTHSM_NS/namespace: $ESIGNET_NS/gperforms a global string replacement that could inadvertently modify data fields if the namespace name appears in the ConfigMap's data. While this is a common pattern for simple K8s resources, consider using a more precise approach for robustness.🔧 Alternative using yq for precise metadata manipulation
if kubectl -n "$SOFTHSM_NS" get configmap esignet-softhsm-share &>/dev/null; then kubectl -n "$SOFTHSM_NS" get configmap esignet-softhsm-share -o yaml | \ - sed "s/namespace: $SOFTHSM_NS/namespace: $ESIGNET_NS/g" | \ + yq eval ".metadata.namespace = \"$ESIGNET_NS\" | del(.metadata.resourceVersion, .metadata.uid, .metadata.creationTimestamp)" | \ kubectl apply -f -Note: This requires
yqto be available in the execution environment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/softhsm-esignet-postinstall.sh` around lines 30 - 38, The sed-based replacement in the block that copies the esignet-softhsm-share ConfigMap is unsafe because it may alter occurrences of the source namespace inside data fields; instead extract the resource and change only metadata.namespace before applying: fetch the ConfigMap (kubectl -n "$SOFTHSM_NS" get configmap esignet-softhsm-share), update .metadata.namespace to "$ESIGNET_NS" using a tool that targets metadata (e.g., yq with an expression that sets metadata.namespace, or kubectl get -o json | jq '.metadata.namespace = env.ESIGNET_NS'), then apply the transformed manifest with kubectl apply -f -; update the code that currently pipes through sed to use this targeted metadata modification so only the namespace field is changed (reference symbols: SOFTHSM_NS, ESIGNET_NS, esignet-softhsm-share, kubectl apply).Helmsman/hooks/esignet-1.7.1/kafka-postinstall.sh (1)
21-26: Consider adding Kafka readiness verification before creating the ConfigMap.Unlike other post-install hooks in this PR (e.g.,
redis-setup.sh), this script creates the ConfigMap immediately without verifying Kafka pods are ready. If this hook runs before Kafka is fully operational, downstream consumers of this ConfigMap may fail.🔧 Proposed fix to add Kafka readiness wait
echo "================================================" echo "eSignet 1.7.1 - Kafka Post-install" echo "================================================" +# --- Wait for Kafka to be ready --- +echo "Waiting for Kafka pods to be ready..." +kubectl -n kafka wait --for=condition=ready pod -l app.kubernetes.io/name=kafka --timeout=300s || \ + echo "WARNING: Kafka pods not ready yet. Continuing with configmap creation." + # --- Create kafka-config configmap in esignet namespace ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/kafka-postinstall.sh` around lines 21 - 26, Add a Kafka readiness wait before creating the ConfigMap in kafka-postinstall.sh: before the kubectl create configmap line, poll or use kubectl wait to ensure Kafka pods are ready (e.g., kubectl wait --for=condition=ready pod -l app=kafka -n "$ESIGNET_NS" --timeout=2m) or loop checking pod status in namespace "$ESIGNET_NS" using the label that identifies your Kafka deployment; log progress, fail the script (exit 1) if pods do not become ready within a reasonable timeout, and only then run the existing kubectl create/apply that uses KAFKA_URL to build the kafka-config ConfigMap.Helmsman/hooks/esignet-1.7.1/esignet-partner-onboarder-preinstall.sh (1)
13-25: Hardcoded namespaces are inconsistent with other hooks.This script uses hardcoded
esignetandkeycloaknamespaces. For consistency with other hooks in this PR that support environment variable overrides (e.g.,ESIGNET_NS), consider parameterizing these namespaces.♻️ Proposed fix
set -euo pipefail +ESIGNET_NS="${ESIGNET_NS:-esignet}" +KEYCLOAK_NS="${KEYCLOAK_NS:-keycloak}" + echo "================================================" echo "eSignet 1.7.1 - Partner Onboarder Pre-install" echo "================================================" # Ensure esignet namespace exists -kubectl create namespace esignet --dry-run=client -o yaml | kubectl apply -f - +kubectl create namespace "$ESIGNET_NS" --dry-run=client -o yaml | kubectl apply -f - # Verify eSignet service is running -kubectl -n esignet wait --for=condition=ready pod -l app.kubernetes.io/name=esignet --timeout=300s 2>/dev/null || \ +kubectl -n "$ESIGNET_NS" wait --for=condition=ready pod -l app.kubernetes.io/name=esignet --timeout=300s 2>/dev/null || \ echo "WARNING: eSignet pods not ready. Partner onboarding may fail." # Verify Keycloak is accessible -if kubectl -n keycloak get svc keycloak &>/dev/null; then +if kubectl -n "$KEYCLOAK_NS" get svc keycloak &>/dev/null; then echo "Keycloak service found." else echo "WARNING: Keycloak service not found. Partner onboarding requires Keycloak." fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/esignet-partner-onboarder-preinstall.sh` around lines 13 - 25, The script currently hardcodes the namespaces "esignet" and "keycloak"; make them configurable by introducing environment variables ESIGNET_NS and KEYCLOAK_NS with defaults (e.g., ESIGNET_NS=${ESIGNET_NS:-esignet}, KEYCLOAK_NS=${KEYCLOAK_NS:-keycloak}) at the top of the script, then replace all occurrences of the literal namespaces in the kubectl invocations (the kubectl create namespace line, the kubectl -n ... wait command that uses app.kubernetes.io/name=esignet, and the kubectl -n keycloak get svc keycloak check) to use the variables ($ESIGNET_NS and $KEYCLOAK_NS) so the hook supports overrides while preserving current defaults..github/workflows/helmsman_mosip.yml (1)
74-86: Shell variables need quoting; profile echo occurs before export.Same issues as
helmsman_testrigs.yml:
- Line 80: Git SHAs should be quoted to prevent potential word splitting.
- Line 85:
$PROFILEis echoed unconditionally but only set in theelsebranch at the shell level.Proposed fix
- name: Set Profile run: | if [ -n "${{ github.event.inputs.profile }}" ]; then + PROFILE="${{ github.event.inputs.profile }}" echo "PROFILE=${{ github.event.inputs.profile }}" >> $GITHUB_ENV else # Auto-detect profile from push trigger — extract profile dir name from changed files - CHANGED_FILES=$(git diff --name-only ${{ github.event.before }} ${{ github.sha }} -- 'Helmsman/dsf/' 2>/dev/null || echo "") + CHANGED_FILES=$(git diff --name-only "${{ github.event.before }}" "${{ github.sha }}" -- 'Helmsman/dsf/' 2>/dev/null || echo "") PROFILE=$(echo "$CHANGED_FILES" | grep 'mosip-dsf.yaml' | head -1 | sed 's|Helmsman/dsf/\([^/]*\)/.*|\1|') PROFILE=${PROFILE:-mosip-platform-java11} echo "PROFILE=$PROFILE" >> $GITHUB_ENV fi echo "Using profile: $PROFILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/helmsman_mosip.yml around lines 74 - 86, The script mis-handles shell quoting and prints PROFILE before it's guaranteed exported; fix the git diff invocation to quote the SHAs (use "${{ github.event.before }}" and "${{ github.sha }}" in the git diff command), quote variable expansions (use "$CHANGED_FILES" when piping/grepping and "$PROFILE" when writing to GITHUB_ENV and echoing), and move the echo "Using profile: $PROFILE" to after the PROFILE is determined and exported to GITHUB_ENV (ensure PROFILE is set in both branches and then echo "$PROFILE").Helmsman/hooks/esignet-1.7.1/esignet-preinstall.sh (1)
9-13: Documented environment variables are not used.The header comments mention
ENABLE_INSECUREandSERVICE_MONITOR_FLAGenvironment variables, but these are not used anywhere in the script. Consider removing them from the documentation or implementing their intended functionality.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/esignet-preinstall.sh` around lines 9 - 13, The header documents ESIGNET_NS, ENABLE_INSECURE and SERVICE_MONITOR_FLAG but only ESIGNET_NS is actually referenced; either remove ENABLE_INSECURE and SERVICE_MONITOR_FLAG from the comment block or implement their behavior: add logic in the script to read ENABLE_INSECURE (e.g., default false) to toggle insecure SSL handling where TLS/curl/helm calls occur, and read SERVICE_MONITOR_FLAG to enable/disable Prometheus service monitor creation; update any usage of ESIGNET_NS, ENABLE_INSECURE, and SERVICE_MONITOR_FLAG in functions or conditional blocks so the documented vars are actually consumed or remove them from the header to keep docs accurate.Helmsman/hooks/esignet-1.7.1/captcha-postinstall.sh (1)
33-38: Namespace substitution viasedmay be fragile.The
sedreplacement on line 37 replaces all occurrences ofnamespace: $ESIGNET_NSin the YAML, not just themetadata.namespacefield. While this works for simple secrets, it could cause issues if the namespace value appears elsewhere in the YAML (e.g., in data values or annotations).Consider using a more targeted approach with
yqorkubectllabel selectors if this becomes problematic, though for basic secrets this pattern is commonly acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/captcha-postinstall.sh` around lines 33 - 38, The sed-based namespace replacement can accidentally change any occurrence of the ESIGNET_NS string in the secret YAML; instead fetch the secret as JSON, set only metadata.namespace, and reapply it. Replace the pipeline that uses sed (the kubectl -n "$ESIGNET_NS" get secret esignet-captcha -o yaml | sed ... | kubectl apply -f -) with a targeted transformation such as kubectl -n "$ESIGNET_NS" get secret esignet-captcha -o json | jq '.metadata.namespace = env.CAPTCHA_NS' | kubectl apply -f - (or use yq to edit .metadata.namespace) so only the metadata.namespace field is changed while keeping CAPTCHA_NS, ESIGNET_NS and the secret name esignet-captcha as the referenced symbols..github/workflows/helmsman_testrigs.yml (1)
74-86: Shell variables need quoting; profile echo occurs before export.Two issues in the "Set Profile" step:
Static analysis flags unquoted variables on line 80 that could cause issues with special characters in commit SHAs (though unlikely in practice).
Line 85
echo "Using profile: $PROFILE"runs beforeGITHUB_ENVexports take effect - the shell variable$PROFILEis only set in theelsebranch but echoed unconditionally. In theifbranch,$PROFILEwill be empty when echoed.Proposed fix
- name: Set Profile run: | if [ -n "${{ github.event.inputs.profile }}" ]; then + PROFILE="${{ github.event.inputs.profile }}" echo "PROFILE=${{ github.event.inputs.profile }}" >> $GITHUB_ENV else # Auto-detect profile from push trigger — extract profile dir name from changed files - CHANGED_FILES=$(git diff --name-only ${{ github.event.before }} ${{ github.sha }} -- 'Helmsman/dsf/' 2>/dev/null || echo "") + CHANGED_FILES=$(git diff --name-only "${{ github.event.before }}" "${{ github.sha }}" -- 'Helmsman/dsf/' 2>/dev/null || echo "") PROFILE=$(echo "$CHANGED_FILES" | grep 'testrigs-dsf.yaml' | head -1 | sed 's|Helmsman/dsf/\([^/]*\)/.*|\1|') PROFILE=${PROFILE:-mosip-platform-java11} echo "PROFILE=$PROFILE" >> $GITHUB_ENV fi echo "Using profile: $PROFILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/helmsman_testrigs.yml around lines 74 - 86, Quote shell variable expansions in the git diff invocation and elsewhere (e.g., use "${{ github.event.before }}" and "${{ github.sha }}" and quote "$CHANGED_FILES" and "$PROFILE") and ensure PROFILE is defined in both branches before printing: in the if branch assign PROFILE="${{ github.event.inputs.profile }}" (then export it to GITHUB_ENV) and in the else branch keep the CHANGED_FILES/PROFILE logic but quote expansions (e.g., CHANGED_FILES="$(git diff --name-only "${{ github.event.before }}" "${{ github.sha }}" -- 'Helmsman/dsf/' 2>/dev/null || echo "")", PROFILE="$(echo "$CHANGED_FILES" | ... )"), then echo "Using profile: $PROFILE" only after PROFILE has been set and exported to GITHUB_ENV; reference CHANGED_FILES, PROFILE, GITHUB_ENV and the git diff/echo "Using profile" lines to locate changes..github/workflows/helmsman_esignet.yml (1)
106-118: Shell variable quoting issue.Line 113 has the same unquoted Git SHA issue flagged by static analysis. The pattern is consistent across workflows.
Proposed fix
- name: Set Profile run: | # Determine profile: from workflow input or detect from changed files on push if [ -n "${{ github.event.inputs.profile }}" ]; then PROFILE="${{ github.event.inputs.profile }}" else # Auto-detect profile from push trigger — extract profile dir name from changed files - CHANGED_FILES=$(git diff --name-only ${{ github.event.before }} ${{ github.sha }} -- 'Helmsman/dsf/' 2>/dev/null || echo "") + CHANGED_FILES=$(git diff --name-only "${{ github.event.before }}" "${{ github.sha }}" -- 'Helmsman/dsf/' 2>/dev/null || echo "") PROFILE=$(echo "$CHANGED_FILES" | grep 'esignet-dsf.yaml' | head -1 | sed 's|Helmsman/dsf/\([^/]*\)/.*|\1|') PROFILE=${PROFILE:-mosip-platform-java11} fi echo "PROFILE=$PROFILE" >> $GITHUB_ENV echo "📁 Using profile: $PROFILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/helmsman_esignet.yml around lines 106 - 118, The git diff call uses unquoted Git SHAs which can break when values contain special characters; update the git diff invocation in the Set Profile step to quote the workflow-expanded SHA variables (the uses of ${{ github.event.before }} and ${{ github.sha }}) so they are passed as single arguments, and ensure any other shell expansions related to CHANGED_FILES and PROFILE (e.g., the CHANGED_FILES assignment and subsequent echo/grep pipeline referencing CHANGED_FILES and PROFILE) remain properly quoted to avoid word-splitting or globbing.Helmsman/hooks/esignet-1.7.1/esignet-init-db.sh (1)
28-30: Consider usingyqfor safer YAML namespace rewriting.The
sedsubstitutions/namespace: $POSTGRES_NS/namespace: $ESIGNET_NS/gcould match unintended occurrences if the namespace string appears elsewhere in the YAML (e.g., in annotations, labels, or data fields). Usingyqwould provide safer, structured manipulation.♻️ Safer alternative using yq
-kubectl -n "$POSTGRES_NS" get secret postgres-postgresql -o yaml | \ - sed "s/namespace: $POSTGRES_NS/namespace: $ESIGNET_NS/g" | \ - kubectl apply -f - +kubectl -n "$POSTGRES_NS" get secret postgres-postgresql -o yaml | \ + yq eval ".metadata.namespace = \"$ESIGNET_NS\"" - | \ + kubectl apply -f -Apply similar changes to lines 35-37 and 41-43.
If
yqisn't available in the execution environment, the current approach should work for standard Kubernetes resources, but add a comment noting the limitation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/hooks/esignet-1.7.1/esignet-init-db.sh` around lines 28 - 30, Replace the brittle sed-based namespace rewrite used in the kubectl pipeline (the block that runs `kubectl -n "$POSTGRES_NS" get secret postgres-postgresql -o yaml | sed "s/namespace: $POSTGRES_NS/namespace: $ESIGNET_NS/g" | kubectl apply -f -`) with a structured yq-based edit to change the metadata.namespace field (and repeat the same yq pattern for the other two similar blocks around lines 35-37 and 41-43) so only the actual namespace field is modified; if yq is not available in the runtime, add a clear comment above the sed pipelines noting the limitation and the risk of accidental matches so future maintainers know to prefer yq when possible..github/workflows/terraform-destroy.yml (1)
44-51: Consider clarifying that empty profile defaults tomosipbehavior.The
INFRA_PROFILEinput hasrequired: falsebutdefault: mosip. If a user explicitly passes an empty string (edge case), the condition[ -n "${{ inputs.INFRA_PROFILE }}" ]at line 118 would be false, falling back to the non-profile path. This is likely fine, but the description could clarify this edge case behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/terraform-destroy.yml around lines 44 - 51, Update the INFRA_PROFILE input description to explicitly state that an empty string (or omission) will result in the default behavior of "mosip" — e.g., mention that the workflow checks the profile with the condition `[ -n "${{ inputs.INFRA_PROFILE }}" ]` and when that check is false it falls back to the non-profile path (effectively defaulting to mosip). Reference the INFRA_PROFILE input and the `[ -n "${{ inputs.INFRA_PROFILE }}" ]` condition so reviewers can find and adjust the descriptive text.docs/profile-based-deployment.md (1)
35-42: Add language specifiers to fenced code blocks for better rendering.The markdownlint tool correctly identifies missing language specifiers. Adding them improves syntax highlighting in documentation viewers.
📝 Add language specifiers
-``` +```text Helmsman/dsf/ ├── prereq-dsf.yamlApply similar changes to code blocks at lines 45, 118, 135, 148, and 164.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/profile-based-deployment.md` around lines 35 - 42, Several fenced code blocks (for example the block beginning with "Helmsman/dsf/" that shows the directory tree) are missing language specifiers; update each triple-backtick fence to include an appropriate language tag (e.g., ```text or ```bash) so markdown renderers and markdownlint treat them correctly. Locate the fenced blocks that display file/directory listings (including the one starting with "Helmsman/dsf/" and the other similar directory-tree/code blocks mentioned) and prepend the language identifier after the opening ``` for each block, keeping the block contents unchanged.Helmsman/dsf/mosip-platform-java21/esignet-dsf.yaml (1)
72-76: Version placeholders need updating for Java 21 profile.The documentation confirms this file is "identical copies of java11; need to update chart versions, image tags, and DB branches for Java 21 / MOSIP 1.3.0". The current
v1.4.1branch values are placeholders.Consider adding a TODO comment at the top of the file to make this explicit:
# TODO: Update versions for Java 21 / MOSIP 1.3.0 # - databases.mosip_esignet.branch: "v1.7.1" (or appropriate version) # - esignet chart version: 1.7.1 # - oidc-ui chart version: 1.7.1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/dsf/mosip-platform-java21/esignet-dsf.yaml` around lines 72 - 76, Add a TODO at the top of this Java 21 profile YAML noting that versions need updating for Java 21 / MOSIP 1.3.0 and replace placeholder branch/version values: update databases.mosip_esignet.branch (currently "v1.4.1") to the correct MOSIP branch (e.g., "v1.7.1" or the approved version), and ensure the esignet chart version and oidc-ui chart version are set to the matching 1.7.1 (or agreed) values; reference the symbols databases.mosip_esignet.branch, esignet chart version, and oidc-ui chart version when making these changes so the intent is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/configure-backend.sh:
- Around line 147-158: The script incorrectly overwrites profile by redeclaring
"local profile" with $3 (which is the branch) after initially setting "local
profile=$4"; to fix, assign positional args explicitly and only once: set "local
branch=${3}" (or "local branch=${3:-}" with a default) and "local profile=${4}"
(keep the original "local profile" assignment), then compute "local
branch_suffix=\"${branch:-local}\"" and build "state_file" using provider,
component, profile and branch_suffix; remove the second "local profile"
re-declaration so profile isn't clobbered.
In @.github/workflows/terraform.yml:
- Around line 148-159: In configure-backend.sh fix the parameter indexing in the
create_local_backend function: the local variable profile is currently assigned
from the wrong positional parameter (local profile="$3") so it gets set to the
branch value; change it to read the fourth parameter (local profile="$4") so the
profile argument passed to configure-backend.sh is preserved and used when
constructing the local state filename.
In `@Helmsman/dsf/esignet/esignet-dsf.yaml`:
- Around line 58-72: The oidc-ui release block is missing wait: true which can
cause later releases to proceed before oidc-ui is ready; update the oidc-ui
chart stanza (the block starting with "oidc-ui:", referencing keys like enabled,
version, chart, set, timeout, priority, hooks) to add wait: true (e.g.,
alongside timeout) so Helm waits for resources to be ready before continuing.
Ensure the wait: true key is added at the same indentation level as timeout and
priority within the oidc-ui block.
In `@Helmsman/dsf/esignet/external-dsf.yaml`:
- Around line 32-44: Add an explicit minio namespace entry under the existing
namespaces mapping to match the apps that reference it: update the namespaces
block to include a "minio:" key and set its protected flag (e.g., protected:
false or true as desired) so that the minio and istio-addons-minio apps have a
declared namespace; modify the same namespaces section that currently lists
postgres, keycloak, softhsm, redis, kafka, captcha and add the minio entry to
ensure consistent protection behavior.
- Around line 326-327: The postInstall hook value contains a trailing space in
the string "$WORKDIR/hooks/s3-setup.sh " which can prevent the shell from
locating the script; update the postInstall entry (postInstall) to remove the
trailing space so it reads "$WORKDIR/hooks/s3-setup.sh" (keep quotes intact) and
scan other hook entries for similar stray spaces.
- Around line 298-327: The priority ordering is reversed: because lower numeric
priority values execute first, ensure the MinIO release runs before the Istio
addons by swapping the priority values on the two releases — update the priority
field for the "minio" release and the "istio-addons-minio" release so that
"minio" has the lower number (executes first) and "istio-addons-minio" has the
higher number; locate the priority keys under the "minio" and
"istio-addons-minio" blocks and change them accordingly (e.g., make minio more
negative than istio-addons-minio).
In `@Helmsman/dsf/mosip-platform-java21/external-dsf.yaml`:
- Around line 81-84: Remove the leading whitespace from the branch value for
databases.mosip_kernel by changing the string assigned to
databases.mosip_kernel.branch from " v1.2.0.2" to "v1.2.0.2" so any resolution
of that branch/tag matches exactly; locate the databases.mosip_kernel.branch
entry in the external-dsf.yaml and update the value accordingly.
In `@Helmsman/dsf/mosip-platform-java21/prereq-dsf.yaml`:
- Line 106: The valuesFile entry currently references a misspelled filename
"rancher-loggig-values.yaml" which will fail chart value loading; update the
valuesFile value to the correct filename "rancher-logging-values.yaml" by
editing the valuesFile key in the prereq-dsf.yaml so it points to
"rancher-logging-values.yaml" instead of "rancher-loggig-values.yaml".
In `@Helmsman/hooks/esignet-1.7.1/mock-identity-system-preinstall.sh`:
- Around line 16-22: The preinstall script currently only warns when the
required ConfigMap "softhsm-mock-identity-system-share" is missing; change the
else branch in mock-identity-system-preinstall.sh so it logs a clear error and
exits non-zero (e.g., exit 1) to fail fast instead of proceeding. Locate the
kubectl check for the configmap and replace the warning-only behavior with an
error message and an exit to stop installation when the ConfigMap is absent.
In `@terraform/implementations/aws/infra/profiles/esignet/aws.tfvars`:
- Around line 62-63: The etcd cluster size is set to 2 via k8s_etcd_node_count
which loses quorum if one node fails; change the value of k8s_etcd_node_count
from 2 to either 1 (for a deliberately lightweight, single-member cluster) or 3
(to tolerate one node failure) in the aws.tfvars so the profile matches the
intended availability level.
In `@terraform/implementations/aws/infra/profiles/mosip/aws.tfvars`:
- Around line 82-84: The DNS lists subdomain_public and subdomain_internal are
missing the API entries referenced elsewhere; update subdomain_public (symbol:
subdomain_public) to include "api" and update subdomain_internal (symbol:
subdomain_internal) to include "api-internal" so Terraform will create the DNS
records used by the DSF manifests (e.g., Helmsman DSF references) and prevent
unresolved hostnames.
---
Outside diff comments:
In @.github/workflows/helmsman_external.yml:
- Around line 262-279: The dispatch payload for the workflow-caller step
currently only sends "mode" so the downstream helmsman_mosip.yml never receives
the triggering profile; update the curl -d payload in the "Trigger helmsman
mosip workflow via API" step to include the profile input (use the workflow's
profile value, e.g. github.event.inputs.profile or pass it into an env like
PROFILE) alongside BRANCH and mode so the JSON becomes
{"ref":"<BRANCH>","inputs":{"mode":"apply","profile":"<PROFILE>"}} and
helmsman_mosip.yml will receive the intended profile.
---
Nitpick comments:
In @.github/workflows/helmsman_esignet.yml:
- Around line 106-118: The git diff call uses unquoted Git SHAs which can break
when values contain special characters; update the git diff invocation in the
Set Profile step to quote the workflow-expanded SHA variables (the uses of ${{
github.event.before }} and ${{ github.sha }}) so they are passed as single
arguments, and ensure any other shell expansions related to CHANGED_FILES and
PROFILE (e.g., the CHANGED_FILES assignment and subsequent echo/grep pipeline
referencing CHANGED_FILES and PROFILE) remain properly quoted to avoid
word-splitting or globbing.
In @.github/workflows/helmsman_mosip.yml:
- Around line 74-86: The script mis-handles shell quoting and prints PROFILE
before it's guaranteed exported; fix the git diff invocation to quote the SHAs
(use "${{ github.event.before }}" and "${{ github.sha }}" in the git diff
command), quote variable expansions (use "$CHANGED_FILES" when piping/grepping
and "$PROFILE" when writing to GITHUB_ENV and echoing), and move the echo "Using
profile: $PROFILE" to after the PROFILE is determined and exported to GITHUB_ENV
(ensure PROFILE is set in both branches and then echo "$PROFILE").
In @.github/workflows/helmsman_testrigs.yml:
- Around line 74-86: Quote shell variable expansions in the git diff invocation
and elsewhere (e.g., use "${{ github.event.before }}" and "${{ github.sha }}"
and quote "$CHANGED_FILES" and "$PROFILE") and ensure PROFILE is defined in both
branches before printing: in the if branch assign PROFILE="${{
github.event.inputs.profile }}" (then export it to GITHUB_ENV) and in the else
branch keep the CHANGED_FILES/PROFILE logic but quote expansions (e.g.,
CHANGED_FILES="$(git diff --name-only "${{ github.event.before }}" "${{
github.sha }}" -- 'Helmsman/dsf/' 2>/dev/null || echo "")", PROFILE="$(echo
"$CHANGED_FILES" | ... )"), then echo "Using profile: $PROFILE" only after
PROFILE has been set and exported to GITHUB_ENV; reference CHANGED_FILES,
PROFILE, GITHUB_ENV and the git diff/echo "Using profile" lines to locate
changes.
In @.github/workflows/terraform-destroy.yml:
- Around line 44-51: Update the INFRA_PROFILE input description to explicitly
state that an empty string (or omission) will result in the default behavior of
"mosip" — e.g., mention that the workflow checks the profile with the condition
`[ -n "${{ inputs.INFRA_PROFILE }}" ]` and when that check is false it falls
back to the non-profile path (effectively defaulting to mosip). Reference the
INFRA_PROFILE input and the `[ -n "${{ inputs.INFRA_PROFILE }}" ]` condition so
reviewers can find and adjust the descriptive text.
In `@docs/profile-based-deployment.md`:
- Around line 35-42: Several fenced code blocks (for example the block beginning
with "Helmsman/dsf/" that shows the directory tree) are missing language
specifiers; update each triple-backtick fence to include an appropriate language
tag (e.g., ```text or ```bash) so markdown renderers and markdownlint treat them
correctly. Locate the fenced blocks that display file/directory listings
(including the one starting with "Helmsman/dsf/" and the other similar
directory-tree/code blocks mentioned) and prepend the language identifier after
the opening ``` for each block, keeping the block contents unchanged.
In `@Helmsman/dsf/mosip-platform-java21/esignet-dsf.yaml`:
- Around line 72-76: Add a TODO at the top of this Java 21 profile YAML noting
that versions need updating for Java 21 / MOSIP 1.3.0 and replace placeholder
branch/version values: update databases.mosip_esignet.branch (currently
"v1.4.1") to the correct MOSIP branch (e.g., "v1.7.1" or the approved version),
and ensure the esignet chart version and oidc-ui chart version are set to the
matching 1.7.1 (or agreed) values; reference the symbols
databases.mosip_esignet.branch, esignet chart version, and oidc-ui chart version
when making these changes so the intent is explicit.
In `@Helmsman/hooks/esignet-1.7.1/captcha-postinstall.sh`:
- Around line 33-38: The sed-based namespace replacement can accidentally change
any occurrence of the ESIGNET_NS string in the secret YAML; instead fetch the
secret as JSON, set only metadata.namespace, and reapply it. Replace the
pipeline that uses sed (the kubectl -n "$ESIGNET_NS" get secret esignet-captcha
-o yaml | sed ... | kubectl apply -f -) with a targeted transformation such as
kubectl -n "$ESIGNET_NS" get secret esignet-captcha -o json | jq
'.metadata.namespace = env.CAPTCHA_NS' | kubectl apply -f - (or use yq to edit
.metadata.namespace) so only the metadata.namespace field is changed while
keeping CAPTCHA_NS, ESIGNET_NS and the secret name esignet-captcha as the
referenced symbols.
In `@Helmsman/hooks/esignet-1.7.1/esignet-init-db.sh`:
- Around line 28-30: Replace the brittle sed-based namespace rewrite used in the
kubectl pipeline (the block that runs `kubectl -n "$POSTGRES_NS" get secret
postgres-postgresql -o yaml | sed "s/namespace: $POSTGRES_NS/namespace:
$ESIGNET_NS/g" | kubectl apply -f -`) with a structured yq-based edit to change
the metadata.namespace field (and repeat the same yq pattern for the other two
similar blocks around lines 35-37 and 41-43) so only the actual namespace field
is modified; if yq is not available in the runtime, add a clear comment above
the sed pipelines noting the limitation and the risk of accidental matches so
future maintainers know to prefer yq when possible.
In `@Helmsman/hooks/esignet-1.7.1/esignet-partner-onboarder-postinstall.sh`:
- Around line 13-21: This script hardcodes the namespace "esignet" causing
inconsistency; add the same namespace fallback used elsewhere by defining
ESIGNET_NS="${ESIGNET_NS:-esignet}" near the top and replace the hardcoded -n
esignet occurrences used by JOB_STATUS assignment and the kubectl logs
invocation with -n "$ESIGNET_NS" (ensure the JOB_STATUS variable and both
kubectl commands reference the new ESIGNET_NS variable).
In `@Helmsman/hooks/esignet-1.7.1/esignet-partner-onboarder-preinstall.sh`:
- Around line 13-25: The script currently hardcodes the namespaces "esignet" and
"keycloak"; make them configurable by introducing environment variables
ESIGNET_NS and KEYCLOAK_NS with defaults (e.g.,
ESIGNET_NS=${ESIGNET_NS:-esignet}, KEYCLOAK_NS=${KEYCLOAK_NS:-keycloak}) at the
top of the script, then replace all occurrences of the literal namespaces in the
kubectl invocations (the kubectl create namespace line, the kubectl -n ... wait
command that uses app.kubernetes.io/name=esignet, and the kubectl -n keycloak
get svc keycloak check) to use the variables ($ESIGNET_NS and $KEYCLOAK_NS) so
the hook supports overrides while preserving current defaults.
In `@Helmsman/hooks/esignet-1.7.1/esignet-preinstall.sh`:
- Around line 9-13: The header documents ESIGNET_NS, ENABLE_INSECURE and
SERVICE_MONITOR_FLAG but only ESIGNET_NS is actually referenced; either remove
ENABLE_INSECURE and SERVICE_MONITOR_FLAG from the comment block or implement
their behavior: add logic in the script to read ENABLE_INSECURE (e.g., default
false) to toggle insecure SSL handling where TLS/curl/helm calls occur, and read
SERVICE_MONITOR_FLAG to enable/disable Prometheus service monitor creation;
update any usage of ESIGNET_NS, ENABLE_INSECURE, and SERVICE_MONITOR_FLAG in
functions or conditional blocks so the documented vars are actually consumed or
remove them from the header to keep docs accurate.
In `@Helmsman/hooks/esignet-1.7.1/kafka-postinstall.sh`:
- Around line 21-26: Add a Kafka readiness wait before creating the ConfigMap in
kafka-postinstall.sh: before the kubectl create configmap line, poll or use
kubectl wait to ensure Kafka pods are ready (e.g., kubectl wait
--for=condition=ready pod -l app=kafka -n "$ESIGNET_NS" --timeout=2m) or loop
checking pod status in namespace "$ESIGNET_NS" using the label that identifies
your Kafka deployment; log progress, fail the script (exit 1) if pods do not
become ready within a reasonable timeout, and only then run the existing kubectl
create/apply that uses KAFKA_URL to build the kafka-config ConfigMap.
In `@Helmsman/hooks/esignet-1.7.1/postgres-preinstall.sh`:
- Around line 36-47: The data YAML in the heredoc currently hardcodes
mosip-version: "1.7.1"; change it to read from an environment/template variable
(e.g., ${MOSIP_VERSION} or a Helm value) so the hook can be reused for future
releases; update the heredoc generation in postgres-preinstall.sh to inject that
variable and ensure any callers/exporters set MOSIP_VERSION (or the
corresponding Helm value) before invoking the script, and validate fallback
behavior if the variable is unset.
In `@Helmsman/hooks/esignet-1.7.1/redis-setup.sh`:
- Around line 14-15: The script inconsistently hardcodes REDIS_NS while
ESIGNET_NS supports env overrides; change REDIS_NS to be configurable by
defining it with a default value (e.g., set REDIS_NS using the pattern used for
ESIGNET_NS) so consumers can override the namespace via environment variable,
and update any references to REDIS_NS in this file (and related
functions/commands) to rely on that variable.
- Around line 21-23: The kubectl readiness wait in redis-setup.sh (the kubectl
-n "$REDIS_NS" wait --for=condition=ready pod -l app.kubernetes.io/name=redis
--timeout=300s command) will cause the script to abort on timeout because of set
-e; to make behavior consistent with other hooks, catch the nonzero exit and log
a warning instead of exiting by appending a fallback that prints a clear WARNING
(e.g., indicating Redis wait timed out and the script is continuing) so the
script continues on readiness failure; update the kubectl wait invocation to
handle failure and emit the warning.
In `@Helmsman/hooks/esignet-1.7.1/softhsm-esignet-postinstall.sh`:
- Around line 30-38: The sed-based replacement in the block that copies the
esignet-softhsm-share ConfigMap is unsafe because it may alter occurrences of
the source namespace inside data fields; instead extract the resource and change
only metadata.namespace before applying: fetch the ConfigMap (kubectl -n
"$SOFTHSM_NS" get configmap esignet-softhsm-share), update .metadata.namespace
to "$ESIGNET_NS" using a tool that targets metadata (e.g., yq with an expression
that sets metadata.namespace, or kubectl get -o json | jq '.metadata.namespace =
env.ESIGNET_NS'), then apply the transformed manifest with kubectl apply -f -;
update the code that currently pipes through sed to use this targeted metadata
modification so only the namespace field is changed (reference symbols:
SOFTHSM_NS, ESIGNET_NS, esignet-softhsm-share, kubectl apply).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e1091cda-7966-4b17-af54-cf0c117d1e0d
📒 Files selected for processing (45)
.github/scripts/configure-backend.sh.github/workflows/helmsman_esignet.yml.github/workflows/helmsman_external.yml.github/workflows/helmsman_mosip.yml.github/workflows/helmsman_testrigs.yml.github/workflows/terraform-destroy.yml.github/workflows/terraform.ymlHelmsman/dsf/esignet/esignet-dsf.yamlHelmsman/dsf/esignet/external-dsf.yamlHelmsman/dsf/esignet/prereq-dsf.yamlHelmsman/dsf/mosip-platform-java11/esignet-dsf.yamlHelmsman/dsf/mosip-platform-java11/external-dsf.yamlHelmsman/dsf/mosip-platform-java11/mosip-dsf.yamlHelmsman/dsf/mosip-platform-java11/prereq-dsf.yamlHelmsman/dsf/mosip-platform-java11/testrigs-dsf.yamlHelmsman/dsf/mosip-platform-java21/esignet-dsf.yamlHelmsman/dsf/mosip-platform-java21/external-dsf.yamlHelmsman/dsf/mosip-platform-java21/mosip-dsf.yamlHelmsman/dsf/mosip-platform-java21/prereq-dsf.yamlHelmsman/dsf/mosip-platform-java21/testrigs-dsf.yamlHelmsman/hooks/esignet-1.7.1/captcha-postinstall.shHelmsman/hooks/esignet-1.7.1/esignet-demo-oidc-partner-onboarder-postinstall.shHelmsman/hooks/esignet-1.7.1/esignet-demo-oidc-partner-onboarder-preinstall.shHelmsman/hooks/esignet-1.7.1/esignet-init-db.shHelmsman/hooks/esignet-1.7.1/esignet-partner-onboarder-postinstall.shHelmsman/hooks/esignet-1.7.1/esignet-partner-onboarder-preinstall.shHelmsman/hooks/esignet-1.7.1/esignet-preinstall.shHelmsman/hooks/esignet-1.7.1/kafka-postinstall.shHelmsman/hooks/esignet-1.7.1/keycloak-postinstall.shHelmsman/hooks/esignet-1.7.1/mock-identity-system-preinstall.shHelmsman/hooks/esignet-1.7.1/mock-relying-party-service-preinstall.shHelmsman/hooks/esignet-1.7.1/mock-relying-party-ui-preinstall.shHelmsman/hooks/esignet-1.7.1/oidc-ui-preinstall.shHelmsman/hooks/esignet-1.7.1/postgres-postinstall.shHelmsman/hooks/esignet-1.7.1/postgres-preinstall.shHelmsman/hooks/esignet-1.7.1/redis-setup.shHelmsman/hooks/esignet-1.7.1/softhsm-esignet-postinstall.shHelmsman/hooks/esignet-1.7.1/softhsm-esignet-setup.shHelmsman/hooks/esignet-1.7.1/softhsm-mock-identity-system-postinstall.shHelmsman/hooks/esignet-1.7.1/softhsm-mock-identity-system-preinstall.shdocs/engineer-review-script.txtdocs/profile-based-deployment.drawiodocs/profile-based-deployment.mdterraform/implementations/aws/infra/profiles/esignet/aws.tfvarsterraform/implementations/aws/infra/profiles/mosip/aws.tfvars
terraform/implementations/aws/infra/profiles/esignet/aws.tfvars
Outdated
Show resolved
Hide resolved
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/scripts/configure-backend.sh (1)
200-205: Extract shared state identifier builder to prevent format drift.Line 200-Line 205, Line 272-Line 277, and Line 320-Line 325 duplicate the same profile-aware naming logic. Centralizing this reduces future inconsistencies.
♻️ Refactor sketch
+build_state_id() { + local component="$1" + local branch="$2" + if [ -n "$PROFILE" ]; then + echo "${CLOUD_PROVIDER}-${component}-${PROFILE}-${branch}" + else + echo "${CLOUD_PROVIDER}-${component}-${branch}" + fi +} ... - local state_key - if [ -n "$PROFILE" ]; then - state_key="${CLOUD_PROVIDER}-${component}-${PROFILE}-${branch}-terraform.tfstate" - else - state_key="${CLOUD_PROVIDER}-${component}-${branch}-terraform.tfstate" - fi + local state_key="$(build_state_id "$component" "$branch")-terraform.tfstate" ... - local state_prefix - if [ -n "$PROFILE" ]; then - state_prefix="terraform/${CLOUD_PROVIDER}-${component}-${PROFILE}-${branch}" - else - state_prefix="terraform/${CLOUD_PROVIDER}-${component}-${branch}" - fi + local state_prefix="terraform/$(build_state_id "$component" "$branch")"Also applies to: 272-277, 320-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/configure-backend.sh around lines 200 - 205, Extract the duplicated profile-aware state key construction into a single helper function (e.g., build_state_key or get_state_key) and replace the three copies with calls to that function; the helper should accept component and branch (and read PROFILE and CLOUD_PROVIDER) and return the same format used currently (when PROFILE is set: "${CLOUD_PROVIDER}-${component}-${PROFILE}-${branch}-terraform.tfstate", otherwise: "${CLOUD_PROVIDER}-${component}-${branch}-terraform.tfstate"), then update usages that set state_key to call the new helper to prevent format drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/configure-backend.sh:
- Around line 78-81: The script reads "$2" unconditionally for the --profile
case causing misparses; update the --profile handler in configure-backend.sh to
first validate that a next argument exists and is not another option (e.g., not
starting with '-') before assigning PROFILE="$2", and if invalid print a clear
error and exit non-zero; ensure you adjust the same switch/case block that
contains the "--profile) PROFILE=... shift 2 ;;", adding the presence and prefix
check and failing fast to avoid silently consuming the wrong token.
---
Nitpick comments:
In @.github/scripts/configure-backend.sh:
- Around line 200-205: Extract the duplicated profile-aware state key
construction into a single helper function (e.g., build_state_key or
get_state_key) and replace the three copies with calls to that function; the
helper should accept component and branch (and read PROFILE and CLOUD_PROVIDER)
and return the same format used currently (when PROFILE is set:
"${CLOUD_PROVIDER}-${component}-${PROFILE}-${branch}-terraform.tfstate",
otherwise: "${CLOUD_PROVIDER}-${component}-${branch}-terraform.tfstate"), then
update usages that set state_key to call the new helper to prevent format drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8516c1f7-8ae5-4045-abfa-c020279d3276
📒 Files selected for processing (1)
.github/scripts/configure-backend.sh
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Helmsman/dsf/esignet/external-dsf.yaml`:
- Around line 63-65: The ConfigMap exposure in external-dsf.yaml is inconsistent
with the hook behavior: update the PostgreSQL host value in the hooks
preInstall/postInstall block so it matches what
Helmsman/hooks/esignet-1.7.1/postgres-preinstall.sh writes to esignet-global
(use esignet-postgres.${INSTALLATION_DOMAIN} instead of
postgres.sandbox.xyz.net), or alternatively modify the hook script
(postgress-preinstall.sh) to write the hostname postgres.sandbox.xyz.net into
esignet-global; ensure the same hostname is used in the hooks entries
(preInstall/postInstall) and in the esignet-global output so consumers see a
single consistent postgres host.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d231a0d0-66c4-4f49-a6ee-086080c49bbd
📒 Files selected for processing (1)
Helmsman/dsf/esignet/external-dsf.yaml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
Helmsman/dsf/esignet/external-dsf.yaml (2)
121-143: Duplicate priority-16may cause non-deterministic ordering.Both
postgres-init-esignet(line 123) andredis(line 141) havepriority: -16. In Helmsman, releases with the same priority may execute in an undefined order. If these releases are truly independent, this is acceptable. However, if there are any implicit dependencies or if deterministic deployment order is desired for debugging/reproducibility, consider assigning distinct priorities.♻️ Suggested fix to differentiate priorities
postgres-init-esignet: ... wait: true timeout: 180 - priority: -16 + priority: -16 hooks: preInstall: "$WORKDIR/hooks/esignet-1.7.1/esignet-init-db.sh" redis: ... wait: true timeout: 480 - priority: -16 + priority: -15.5Or adjust redis to
-15and shift subsequent priorities accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/dsf/esignet/external-dsf.yaml` around lines 121 - 143, The two releases 'postgres-init-esignet' and 'redis' both use the same priority value (-16), which can lead to non-deterministic ordering; to fix, change one of the priorities (e.g., set the 'redis' release priority to -15 or adjust one of them to a unique value) so each release has a distinct priority, and if you shift priorities ensure any dependent releases after 'redis' are adjusted accordingly to preserve intended ordering.
73-73: Hardcodedsandbox.xyz.netdomain reduces environment portability.Multiple host values are hardcoded with
sandbox.xyz.net:
- Line 73:
postgresHost- Lines 111, 117:
databases.*.host- Line 208:
kafkaUiHost- Line 275:
keycloakExternalHost- Line 325:
externalHost(minio)The hook scripts use
${INSTALLATION_DOMAIN}for portability. Consider using environment variable substitution in the DSF to maintain consistency and enable deployment to different environments without file modifications.♻️ Example using environment variables
istio-addons-psql: ... set: - postgresHost: "postgres.sandbox.xyz.net" + postgresHost: "postgres.$INSTALLATION_DOMAIN"Apply similar changes to other hardcoded hosts. Ensure
INSTALLATION_DOMAINis exported in the workflow before invoking Helmsman.Also applies to: 111-112, 208-208, 275-275, 325-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/dsf/esignet/external-dsf.yaml` at line 73, Replace hardcoded sandbox.sandbox.xyz.net domain values with environment-variable substitution so the DSF is portable: update postgresHost, databases.*.host, kafkaUiHost, keycloakExternalHost, and externalHost (minio) to use the same INSTALLATION_DOMAIN substitution pattern used in the hook scripts (e.g. reference ${INSTALLATION_DOMAIN} or the project's established envsubst pattern) and ensure the CI/workflow exports INSTALLATION_DOMAIN before Helmsman runs; apply the same substitution for the entries at the reported keys (postgresHost, databases.*.host, kafkaUiHost, keycloakExternalHost, externalHost).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Helmsman/dsf/esignet/external-dsf.yaml`:
- Around line 32-46: Add an explicit esignet namespace entry under the top-level
namespaces block in external-dsf.yaml so it matches how other namespaces are
declared; specifically add an "esignet:" key with an appropriate "protected:"
boolean (e.g., protected: false) to ensure consistent protection settings for
the postgres-init-esignet app (which uses namespace: esignet) and to align with
the namespaces handling elsewhere in the file.
In `@Helmsman/dsf/mosip-platform-java21/external-dsf.yaml`:
- Line 65: The dbUserPasswords.dbuserPassword entry is set to an empty string;
update the external-dsf YAML so dbUserPasswords.dbuserPassword is either
populated with a secure value or clearly linked to your secret management
mechanism (e.g., reference to a Kubernetes Secret, HashiCorp Vault, or an
injection placeholder), and add a short inline comment explaining that the
actual password is provided externally if that’s intentional to avoid accidental
deployment with a blank DB password.
- Around line 303-307: The postInstall hook currently uses literal placeholder
arguments to captcha-setup.sh (postInstall and captcha-setup.sh), which will
create invalid Kubernetes secrets if not replaced; update the manifest to pass
environment-variable references (e.g., $PREREG_SITE_KEY, $PREREG_SECRET_KEY,
etc.) instead of angle-bracket literals and/or modify captcha-setup.sh to
validate inputs (reject values matching placeholder patterns like "<.*>" or
empty/unset values and exit non‑zero) so deployment fails fast when real keys
are missing.
- Line 270: The postInstall entry contains an unintended trailing space in the
hook path ("$WORKDIR/hooks/s3-setup.sh "); remove the trailing space from the
postInstall value so the hook reads exactly "$WORKDIR/hooks/s3-setup.sh" (update
the postInstall key where it is defined in external-dsf.yaml).
- Line 392: The configuration key landing.name currently contains a literal
placeholder value "<sandbox>" which will render with angle brackets on the UI;
change landing.name to a final display string without angle brackets (e.g.,
"Sandbox") or replace it with a templated/env var pattern such as using a
variable token (e.g., landing.name: "${LANDING_NAME}") that will be substituted
at deployment so the UI shows the intended text rather than the literal
"<sandbox>".
---
Nitpick comments:
In `@Helmsman/dsf/esignet/external-dsf.yaml`:
- Around line 121-143: The two releases 'postgres-init-esignet' and 'redis' both
use the same priority value (-16), which can lead to non-deterministic ordering;
to fix, change one of the priorities (e.g., set the 'redis' release priority to
-15 or adjust one of them to a unique value) so each release has a distinct
priority, and if you shift priorities ensure any dependent releases after
'redis' are adjusted accordingly to preserve intended ordering.
- Line 73: Replace hardcoded sandbox.sandbox.xyz.net domain values with
environment-variable substitution so the DSF is portable: update postgresHost,
databases.*.host, kafkaUiHost, keycloakExternalHost, and externalHost (minio) to
use the same INSTALLATION_DOMAIN substitution pattern used in the hook scripts
(e.g. reference ${INSTALLATION_DOMAIN} or the project's established envsubst
pattern) and ensure the CI/workflow exports INSTALLATION_DOMAIN before Helmsman
runs; apply the same substitution for the entries at the reported keys
(postgresHost, databases.*.host, kafkaUiHost, keycloakExternalHost,
externalHost).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 97e179eb-eb8e-4964-82a3-01a6b7051ae1
📒 Files selected for processing (2)
Helmsman/dsf/esignet/external-dsf.yamlHelmsman/dsf/mosip-platform-java21/external-dsf.yaml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
Helmsman/dsf/esignet/external-dsf.yaml (1)
69-77:⚠️ Potential issue | 🟠 MajorUse one PostgreSQL endpoint for initialization and runtime config.
Helmsman/hooks/esignet-1.7.1/postgres-postinstall.shdefaults topostgres-postgresql.postgres:5432, but Lines 113-114 route the init chart throughpostgres.sandbox.xyz.net:5433. Since the init job runs inside the cluster, this adds a DNS/ingress dependency and leaves the DSF with conflicting PostgreSQL targets. The same mismatch is already baked into Lines 119-121 ifmosip_mockidentitysystemis ever enabled.🛠️ Align the init jobs with the in-cluster PostgreSQL service
postgres-init-esignet: @@ - databases.mosip_esignet.host: "postgres.sandbox.xyz.net" - databases.mosip_esignet.port: 5433 + databases.mosip_esignet.host: "postgres-postgresql.postgres" + databases.mosip_esignet.port: 5432 @@ - databases.mosip_mockidentitysystem.host: "postgres.sandbox.xyz.net" + databases.mosip_mockidentitysystem.host: "postgres-postgresql.postgres" @@ - databases.mosip_mockidentitysystem.port: 5433 + databases.mosip_mockidentitysystem.port: 5432Also applies to: 111-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/dsf/esignet/external-dsf.yaml` around lines 69 - 77, The init job and runtime chart are using different PostgreSQL endpoints causing conflicting targets: update the istio-addons-psql chart values so the postgresHost/port matches the in-cluster service used by the init script (Helmsman/hooks/esignet-1.7.1/postgres-postinstall.sh which expects postgres-postgresql.postgres:5432) — change the postgresHost from "postgres.sandbox.xyz.net" and port 5433 (where present) to the in-cluster DNS "postgres-postgresql.postgres:5432" (and ensure any conditional block for mosip_mockidentitysystem likewise uses the same postgresHost/port) so both initialization (postgres-postinstall.sh) and runtime charts reference one consistent in-cluster PostgreSQL endpoint.
🧹 Nitpick comments (3)
Helmsman/dsf/mosip-platform-java21/prereq-dsf.yaml (3)
36-37: Placeholder values require environment-specific substitution.The hook parameters contain placeholders (
sandbox.xyz.net,<version>) that must be replaced with actual values before deployment. If these are meant to be dynamically substituted, consider using environment variables (e.g.,$DOMAIN,$ISTIO_VERSION) for consistency with the$WORKDIRpattern used elsewhere.♻️ Suggested approach
hooks: - postInstall: "$WORKDIR/hooks/install-istio-and-httpbin.sh sandbox.xyz.net <version>" - postUpgrade: "$WORKDIR/hooks/install-istio-and-httpbin.sh sandbox.xyz.net <version>" + postInstall: "$WORKDIR/hooks/install-istio-and-httpbin.sh $DOMAIN $ISTIO_VERSION" + postUpgrade: "$WORKDIR/hooks/install-istio-and-httpbin.sh $DOMAIN $ISTIO_VERSION"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/dsf/mosip-platform-java21/prereq-dsf.yaml` around lines 36 - 37, The postInstall and postUpgrade hook lines use hardcoded placeholders ("sandbox.xyz.net" and "<version>")—update these to use environment-specific substitution by replacing the placeholders with environment variables (for example $DOMAIN and $ISTIO_VERSION) consistent with the existing $WORKDIR pattern; modify the two entries referencing $WORKDIR/hooks/install-istio-and-httpbin.sh so they pass "$DOMAIN" and "$ISTIO_VERSION" (or other agreed variable names) instead of the literal placeholders and ensure any documentation or deployment scripts set those env vars prior to running the helm chart.
88-89: Hardcoded Kibana host should be parameterized.
kibanaHost: "kibana.sandbox.xyz.net"is environment-specific and should use an environment variable for portability across deployments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/dsf/mosip-platform-java21/prereq-dsf.yaml` around lines 88 - 89, The kibanaHost entry is hardcoded; replace the literal "kibana.sandbox.xyz.net" with a parameter reference (e.g. a Helm/chart/value or environment variable) so deployments can supply the host per environment; update the key kibanaHost to use the templated value (for example {{ .Values.kibanaHost }} or ${KIBANA_HOST}) and ensure the corresponding default is added to values.yaml or documented in the deployment env variables alongside installName to maintain backward compatibility.
6-9: Remove unused and deprecated repository reference.The
banzaicloud-stablerepository points tohttps://charts.helm.sh/stable, which is the deprecated and archived Helm stable repository (read-only since November 2020). Additionally, no charts in this file reference this repository—all usemosip/charts or local paths. Since this configuration also appears identically in other DSF profiles (java11, esignet), consider removing this unused repository across all affected files.If a Banzaicloud repository is needed in the future, use the correct URL instead of the deprecated Helm stable archive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Helmsman/dsf/mosip-platform-java21/prereq-dsf.yaml` around lines 6 - 9, Remove the unused/deprecated helm repo entry by deleting the "banzaicloud-stable: https://charts.helm.sh/stable" line from the helmRepos block (symbol: helmRepos) in this file and the other DSF profile YAMLs that contain the identical entry (symbols: banzaicloud-stable, https://charts.helm.sh/stable); ensure only active repositories (e.g., mosip and bitnami) remain, run a quick grep across DSF profiles to locate other instances, and commit the cleaned YAMLs so no references to the deprecated Helm stable archive remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Helmsman/dsf/esignet/external-dsf.yaml`:
- Around line 209-210: The esignet profile currently hard-codes sandbox hosts
(e.g., the kafkaUiHost key set to "kafka.sandbox.xyz.net"); remove those literal
hostnames and source them from profile/installation inputs instead (e.g., use
the installation-domain/profile-provided value or a templated variable) so the
DSF remains environment-agnostic; update the kafkaUiHost entries in the esignet
profile (and the same pattern used for the other host keys in that profile) to
reference the appropriate input variable (installation-domain or
profile-specific input) rather than a fixed "sandbox.xyz.net" string.
- Around line 160-162: Find every occurrence of the YAML key logRetentionBytes
in the DSF config files and remove the leading underscore from its value (e.g.,
change "_1073741824" to "1073741824") so the value is a valid integer string for
Kafka's log.retention.bytes; update all three DSF configs that contain this key
to use the corrected value.
In `@Helmsman/dsf/mosip-platform-java21/prereq-dsf.yaml`:
- Around line 67-74: The configured JVM heap sizes (data.heapSize and
master.heapSize) exceed their Kubernetes memory requests; update either the heap
settings or the memory requests so requests > heap (recommend requests >= 1.5x
heap to allow JVM native/metaspace overhead). Specifically, adjust
data.resources.requests.memory and master.resources.requests.memory to values at
least 1.5 times data.heapSize and master.heapSize respectively (or reduce
data.heapSize/master.heapSize to fit the current requests) so the memory request
always exceeds the configured heap.
---
Duplicate comments:
In `@Helmsman/dsf/esignet/external-dsf.yaml`:
- Around line 69-77: The init job and runtime chart are using different
PostgreSQL endpoints causing conflicting targets: update the istio-addons-psql
chart values so the postgresHost/port matches the in-cluster service used by the
init script (Helmsman/hooks/esignet-1.7.1/postgres-postinstall.sh which expects
postgres-postgresql.postgres:5432) — change the postgresHost from
"postgres.sandbox.xyz.net" and port 5433 (where present) to the in-cluster DNS
"postgres-postgresql.postgres:5432" (and ensure any conditional block for
mosip_mockidentitysystem likewise uses the same postgresHost/port) so both
initialization (postgres-postinstall.sh) and runtime charts reference one
consistent in-cluster PostgreSQL endpoint.
---
Nitpick comments:
In `@Helmsman/dsf/mosip-platform-java21/prereq-dsf.yaml`:
- Around line 36-37: The postInstall and postUpgrade hook lines use hardcoded
placeholders ("sandbox.xyz.net" and "<version>")—update these to use
environment-specific substitution by replacing the placeholders with environment
variables (for example $DOMAIN and $ISTIO_VERSION) consistent with the existing
$WORKDIR pattern; modify the two entries referencing
$WORKDIR/hooks/install-istio-and-httpbin.sh so they pass "$DOMAIN" and
"$ISTIO_VERSION" (or other agreed variable names) instead of the literal
placeholders and ensure any documentation or deployment scripts set those env
vars prior to running the helm chart.
- Around line 88-89: The kibanaHost entry is hardcoded; replace the literal
"kibana.sandbox.xyz.net" with a parameter reference (e.g. a Helm/chart/value or
environment variable) so deployments can supply the host per environment; update
the key kibanaHost to use the templated value (for example {{ .Values.kibanaHost
}} or ${KIBANA_HOST}) and ensure the corresponding default is added to
values.yaml or documented in the deployment env variables alongside installName
to maintain backward compatibility.
- Around line 6-9: Remove the unused/deprecated helm repo entry by deleting the
"banzaicloud-stable: https://charts.helm.sh/stable" line from the helmRepos
block (symbol: helmRepos) in this file and the other DSF profile YAMLs that
contain the identical entry (symbols: banzaicloud-stable,
https://charts.helm.sh/stable); ensure only active repositories (e.g., mosip and
bitnami) remain, run a quick grep across DSF profiles to locate other instances,
and commit the cleaned YAMLs so no references to the deprecated Helm stable
archive remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8a017482-eda7-4492-98fb-2fdf32d151d6
📒 Files selected for processing (3)
Helmsman/dsf/esignet/external-dsf.yamlHelmsman/dsf/mosip-platform-java21/prereq-dsf.yamlHelmsman/utils/rancher-loggig-values.yaml
💤 Files with no reviewable changes (1)
- Helmsman/utils/rancher-loggig-values.yaml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
Signed-off-by: bhumi46 <thisisbn46@gmail.com>
bhumi46
added a commit
to bhumi46/infra
that referenced
this pull request
Mar 26, 2026
Merge pull request mosip#173 from bhumi46/develop
This was referenced Mar 27, 2026
Open
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
1 participant
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
Removed
Documentation