feat: implement flow mode for interactive commands in captain_utils.sh#458
feat: implement flow mode for interactive commands in captain_utils.sh#458hamzabouissi wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a “flow mode” to captain_utils.sh so interactive gum prompts can be driven non-interactively via a queued set of responses.
Changes:
- Introduces flow-mode infrastructure (
--flow) with wrappers forgum choose,gum confirm, andgum pager. - Replaces existing interactive prompts/pager usage with
flow_choose,flow_confirm, andflow_pager. - Adds a helm diff + confirmation step to the Calico upgrade path before applying changes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| helm_diff_cmd="helm diff --color upgrade \"$component\" \"$chart_name\" --version \"$version\" -f \"$target_file\" -n \"$namespace\" --allow-unreleased" | ||
|
|
||
| # New: Select ArgoCD CRD version if argocd is chosen | ||
| gum style --bold --foreground 212 "Select ArgoCD App Version:" | ||
| if [ "$environment" = "production" ]; then | ||
| local argocd_crd_versions=`yq '.versions[] | select(.name == "argocd_app_version") | .version' VERSIONS/glueops.yaml` | ||
| else | ||
| local argocd_crd_versions=`v($(helm search repo argo/argo-cd --versions -o json | jq --arg chart_helm_version "$version" -r '.[] | select(.version == $chart_helm_version).app_version' | sed 's/^v//'))` | ||
| fi | ||
| chosen_crd_version=$(gum choose "${argocd_crd_versions[@]}" "Back") | ||
| chosen_crd_version=$(flow_choose "${argocd_crd_versions[@]}" "Back") | ||
| pre_commands="kubectl apply -k \"https://github.com/argoproj/argo-cd/manifests/crds?ref=$chosen_crd_version\" && helm repo update" | ||
| # Check if user wants to go back | ||
| if [ "$chosen_crd_version" = "Back" ]; then | ||
| return | ||
| fi | ||
|
|
||
| set -x | ||
| eval "$helm_diff_cmd | gum pager" # Execute the main helm diff command | ||
| eval "$helm_diff_cmd" | flow_pager # Execute the main helm diff command | ||
| gum style --bold --foreground 212 "✅ Diff complete." |
There was a problem hiding this comment.
Same concern here: eval "$helm_diff_cmd" | flow_pager relies on eval for command execution. Building the helm diff command as an argv array and executing it directly will be safer and remove the need for embedded quoting inside helm_diff_cmd.
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| --flow) | ||
| FLOW_MODE=true | ||
| IFS=',' read -r -a FLOW_QUEUE <<< "$2" | ||
| shift 2 | ||
| ;; |
There was a problem hiding this comment.
--flow option parsing assumes a second argument exists. With set -u/set -e, running captain_utils.sh --flow (missing queue) will error via an unbound $2 and/or shift 2, instead of showing a helpful usage message. Add an explicit [[ $# -ge 2 ]] guard (and ideally validate non-empty) before reading $2 and shifting.
| _flow_set_index() { echo "$1" > "$FLOW_INDEX_FILE"; } | ||
|
|
||
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| --flow) | ||
| FLOW_MODE=true | ||
| IFS=',' read -r -a FLOW_QUEUE <<< "$2" |
There was a problem hiding this comment.
Flow queue parsing doesn’t trim whitespace around comma-separated values. A common input like --flow "1, 2,3" produces an element with a leading space (" 2"), which will fail the numeric validation later. Consider stripping spaces (e.g., remove whitespace after splitting, or set IFS=', ' with additional normalization) so flow mode is resilient to typical CLI formatting.
| _flow_set_index() { echo "$1" > "$FLOW_INDEX_FILE"; } | |
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --flow) | |
| FLOW_MODE=true | |
| IFS=',' read -r -a FLOW_QUEUE <<< "$2" | |
| _flow_set_index() { echo "$1" > "$FLOW_INDEX_FILE"; } | |
| _trim_whitespace() { | |
| local value="$1" | |
| value="${value#"${value%%[![:space:]]*}"}" | |
| value="${value%"${value##*[![:space:]]}"}" | |
| printf '%s' "$value" | |
| } | |
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --flow) | |
| FLOW_MODE=true | |
| IFS=',' read -r -a FLOW_QUEUE <<< "$2" | |
| for i in "${!FLOW_QUEUE[@]}"; do | |
| FLOW_QUEUE[$i]=$(_trim_whitespace "${FLOW_QUEUE[$i]}") | |
| done |
| if [ "$val" = "y" ] || [ "$val" = "Y" ]; then | ||
| return 0 | ||
| else | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
In flow mode, flow_confirm treats any value other than y/Y as “no” without validating the input, even though the comment says the queue value is (y/n). This can silently mask a malformed flow queue (e.g., yes, 0, empty). Consider validating accepted values (y|Y|n|N) and exiting with a clear error on anything else.
| if [ "$val" = "y" ] || [ "$val" = "Y" ]; then | |
| return 0 | |
| else | |
| return 1 | |
| fi | |
| case "$val" in | |
| y|Y) | |
| return 0 | |
| ;; | |
| n|N) | |
| return 1 | |
| ;; | |
| *) | |
| echo "Invalid flow confirm value '$val'. Expected one of: y, Y, n, N." >&2 | |
| exit 1 | |
| ;; | |
| esac |
| helm_diff_cmd="helm diff --color upgrade \"$component\" \"$chart_name\" --version \"$version\" -f \"$target_file\" -f \"$overrides_file\" -n \"$namespace\" --allow-unreleased" | ||
|
|
||
| set -x | ||
| eval "$helm_diff_cmd | gum pager" # Execute the main helm diff command | ||
| eval "$helm_diff_cmd" | flow_pager # Execute the main helm diff command | ||
| gum style --bold --foreground 212 "✅ Diff complete." |
There was a problem hiding this comment.
This uses eval to execute a command string (helm_diff_cmd) built from variables. Even if inputs are usually trusted, eval makes quoting brittle and can introduce command-injection risk if any variable contains unexpected characters. Prefer constructing the helm diff invocation as an array and executing it directly (then pipe to flow_pager) to avoid eval entirely.
No description provided.