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 3 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.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| --flow) | ||
| FLOW_MODE=true | ||
| IFS=',' read -r -a FLOW_QUEUE <<< "$2" | ||
| shift 2 | ||
| ;; | ||
| *) | ||
| echo "Unknown option: $1" >&2 |
There was a problem hiding this comment.
--flow assumes a second argument exists ($2) and then shift 2. With set -u, calling the script with --flow but no queue will terminate with an unhelpful “unbound variable” / shift error. Please validate # of remaining args before reading $2 and print a clear usage message (or support --flow=<queue>).
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --flow) | |
| FLOW_MODE=true | |
| IFS=',' read -r -a FLOW_QUEUE <<< "$2" | |
| shift 2 | |
| ;; | |
| *) | |
| echo "Unknown option: $1" >&2 | |
| usage() { | |
| echo "Usage: $(basename "$0") [--flow <queue>]" >&2 | |
| } | |
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --flow) | |
| if [[ $# -lt 2 ]]; then | |
| echo "Error: --flow requires a queue argument." >&2 | |
| usage | |
| exit 1 | |
| fi | |
| FLOW_MODE=true | |
| IFS=',' read -r -a FLOW_QUEUE <<< "$2" | |
| shift 2 | |
| ;; | |
| *) | |
| echo "Unknown option: $1" >&2 | |
| usage |
| helm repo add incubating-glueops-project-template https://incubating-helm.gpkg.io/project-template | ||
| helm repo add projectcalico https://docs.tigera.io/calico/charts | ||
| helm repo update | ||
| helm plugin install https://github.com/databus23/helm-diff |
There was a problem hiding this comment.
helm plugin install https://github.com/databus23/helm-diff will typically exit non-zero if the plugin is already installed, and because the script uses set -e this can make subsequent runs fail. Please make this idempotent (e.g., check helm plugin list first, or attempt install and fall back to update).
| helm plugin install https://github.com/databus23/helm-diff | |
| if helm plugin list | awk 'NR > 1 {print $1}' | grep -qx 'diff'; then | |
| helm plugin update diff | |
| else | |
| helm plugin install https://github.com/databus23/helm-diff | |
| fi |
| 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") |
There was a problem hiding this comment.
In the non-production branch, local argocd_crd_versions=v($(helm search repo ...))`` invokes a v command/function that is not defined anywhere in this script (and aliases won’t expand in non-interactive shells). With `set -e`, selecting ArgoCD in `dev` will fail here. Please remove/fix the `v(...)` wrapper and ensure `argocd_crd_versions` is produced as a proper array of options before calling `flow_choose`.
| set -x | ||
| helm diff --color upgrade calico projectcalico/tigera-operator --version ${calico_version} --namespace tigera-operator -f calico.yaml --allow-unreleased | flow_pager | ||
| gum style --bold --foreground 212 "✅ Diff complete." |
There was a problem hiding this comment.
The new Calico diff/upgrade commands use unquoted ${calico_version}. If the value contains whitespace/newlines (e.g., from yq output), it can lead to word-splitting and incorrect helm arguments. Please quote the variable expansions in these helm invocations.
No description provided.