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 5 commits intomainfrom
Conversation
github-actions
Bot
added
include-in-release-notes
enhancement
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.
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.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Use a temp file for FLOW_INDEX so it persists across subshells created by $() | ||
| FLOW_INDEX_FILE=$(mktemp) | ||
| echo 0 > "$FLOW_INDEX_FILE" | ||
| trap 'rm -f "$FLOW_INDEX_FILE"' EXIT |
There was a problem hiding this comment.
FLOW_INDEX_FILE is created unconditionally at startup even when --flow isn’t used. This adds needless temp-file churn and makes behavior less predictable if the script is sourced. Consider creating the temp file (and installing the trap) only when FLOW_MODE is enabled.
| 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 |
There was a problem hiding this comment.
This still relies on eval to run the constructed helm_diff_cmd. Since version/other fields can come from external commands (yq, gh release list, etc.), eval can mis-handle quoting or allow unintended shell interpretation. Prefer building the command as an array and executing it directly (and then piping to flow_pager) to avoid injection/quoting bugs.
| helm repo add projectcalico https://docs.tigera.io/calico/charts | ||
| helm repo update |
There was a problem hiding this comment.
handle_calico_upgrades re-runs helm repo add projectcalico + helm repo update even though run_prerequisite_commands already adds that repo and updates at script startup. This adds latency to every calico run; consider removing the duplicate update here or guarding it (e.g. only add if missing, and avoid an extra update unless needed).
| helm repo add projectcalico https://docs.tigera.io/calico/charts | |
| helm repo update | |
| if ! helm repo list | awk '{print $1}' | grep -Fxq projectcalico; then | |
| helm repo add projectcalico https://docs.tigera.io/calico/charts | |
| helm repo update | |
| fi |
| exit 1 | ||
| fi | ||
| local idx="${FLOW_QUEUE[$flow_index]}" | ||
| _flow_set_index $((flow_index + 1)) | ||
| if ! [[ "$idx" =~ ^[0-9]+$ ]] || [ "$idx" -lt 1 ] || [ "$idx" -gt "${#options[@]}" ]; then | ||
| echo "Invalid flow index '$idx'. Valid range: 1-${#options[@]} for options: ${options[*]}" >&2 | ||
| exit 1 |
There was a problem hiding this comment.
flow_choose calls exit when the flow queue is exhausted/invalid. Because callers use command substitution (var=$(flow_choose ...)), exit will terminate the entire script immediately and bypass any caller-side handling like || exit 0. Prefer return 1 (and maybe echo an error to stderr) so the caller can decide whether to exit, retry, or go back.
| exit 1 | |
| fi | |
| local idx="${FLOW_QUEUE[$flow_index]}" | |
| _flow_set_index $((flow_index + 1)) | |
| if ! [[ "$idx" =~ ^[0-9]+$ ]] || [ "$idx" -lt 1 ] || [ "$idx" -gt "${#options[@]}" ]; then | |
| echo "Invalid flow index '$idx'. Valid range: 1-${#options[@]} for options: ${options[*]}" >&2 | |
| exit 1 | |
| return 1 | |
| fi | |
| local idx="${FLOW_QUEUE[$flow_index]}" | |
| _flow_set_index $((flow_index + 1)) | |
| if ! [[ "$idx" =~ ^[0-9]+$ ]] || [ "$idx" -lt 1 ] || [ "$idx" -gt "${#options[@]}" ]; then | |
| echo "Invalid flow index '$idx'. Valid range: 1-${#options[@]} for options: ${options[*]}" >&2 | |
| return 1 |
| local flow_index=$(_flow_get_index) | ||
| if [ "$flow_index" -ge "${#FLOW_QUEUE[@]}" ]; then | ||
| echo "Flow complete." >&2 | ||
| exit 0 |
There was a problem hiding this comment.
flow_confirm exits the script (exit 0) when the flow queue is exhausted. This makes flow_confirm unsafe to use in conditionals (e.g. if ! flow_confirm ...) because it can terminate the whole script mid-function. Prefer returning a status (e.g. return 1/return 0) and let the caller decide what to do when the flow runs out.
| exit 0 | |
| return 1 |
No description provided.