Skip to content

feat: implement flow mode for interactive commands in captain_utils.sh#458

Open
hamzabouissi wants to merge 2 commits intomainfrom
feat/add_flow_mode
Open

feat: implement flow mode for interactive commands in captain_utils.sh#458
hamzabouissi wants to merge 2 commits intomainfrom
feat/add_flow_mode

Conversation

@hamzabouissi
Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings April 14, 2026 12:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 for gum choose, gum confirm, and gum pager.
  • Replaces existing interactive prompts/pager usage with flow_choose, flow_confirm, and flow_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.

Comment on lines 203 to 221
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."
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +26
while [[ $# -gt 0 ]]; do
case "$1" in
--flow)
FLOW_MODE=true
IFS=',' read -r -a FLOW_QUEUE <<< "$2"
shift 2
;;
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--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.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +24
_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"
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
_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

Copilot uses AI. Check for mistakes.
Comment on lines +77 to +81
if [ "$val" = "y" ] || [ "$val" = "Y" ]; then
return 0
else
return 1
fi
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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

Copilot uses AI. Check for mistakes.
Comment on lines 158 to 162
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."
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants