From b2dd67a0afa01d997a5d054dd02ecbdc0a57ce28 Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Fri, 13 Feb 2026 05:07:53 -0800 Subject: [PATCH] refactor: extract helpers to reduce complexity in fly and netcup providers (#912) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fly/lib/common.sh: - Extract _get_fly_cmd() to eliminate duplicated fly/flyctl CLI resolution across run_server, interactive_session, _try_flyctl_auth, ensure_fly_cli - Extract _fly_parse_error() to deduplicate JSON error parsing (was inline in _validate_fly_token, _fly_create_app, _fly_create_machine) - Extract _fly_build_machine_body() from _fly_create_machine (50→32 lines) - Use shared _extract_json_field in _fly_create_machine and _fly_wait_for_machine_start instead of inline python3 calls netcup/lib/common.sh: - Extract _netcup_is_success() for repeated status=='success' checks (was inline python3 in create_server, destroy_server, _netcup_wait_for_ip) - Extract _netcup_build_login_body() from netcup_get_session (51→30 lines) - Use _extract_json_field throughout instead of inline python3 one-liners - Net reduction: 351→335 lines (-16) Agent: complexity-hunter Co-authored-by: A <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- fly/lib/common.sh | 108 ++++++++++++++++++++++--------------------- netcup/lib/common.sh | 84 ++++++++++++++------------------- 2 files changed, 90 insertions(+), 102 deletions(-) diff --git a/fly/lib/common.sh b/fly/lib/common.sh index 6d4ce1da..16e272d3 100644 --- a/fly/lib/common.sh +++ b/fly/lib/common.sh @@ -33,19 +33,31 @@ fly_api() { generic_cloud_api "$FLY_API_BASE" "$FLY_API_TOKEN" "$method" "$endpoint" "$body" } +# Resolve the flyctl CLI command name ("fly" or "flyctl") +# Prints the command name on stdout; returns 1 if neither is found +_get_fly_cmd() { + if command -v fly &>/dev/null; then + echo "fly" + elif command -v flyctl &>/dev/null; then + echo "flyctl" + else + return 1 + fi +} + +# Parse the "error" field from a Fly.io API JSON response +# Usage: echo "$response" | _fly_parse_error [DEFAULT] +_fly_parse_error() { + local default="${1:-Unknown error}" + python3 -c "import json,sys; d=json.loads(sys.stdin.read()); print(d.get('error',sys.argv[1]))" "$default" 2>/dev/null || cat +} + # Ensure flyctl CLI is installed ensure_fly_cli() { - if command -v fly &>/dev/null; then + if _get_fly_cmd &>/dev/null; then log_info "flyctl CLI available" return 0 fi - if command -v flyctl &>/dev/null; then - log_info "flyctl CLI available (as flyctl)" - # Create alias function so we can use 'fly' consistently - fly() { flyctl "$@"; } - export -f fly - return 0 - fi log_step "Installing flyctl CLI..." curl -L https://fly.io/install.sh | sh 2>/dev/null || { @@ -59,7 +71,7 @@ ensure_fly_cli() { export PATH="$HOME/.fly/bin:$PATH" fi - if ! command -v fly &>/dev/null && ! command -v flyctl &>/dev/null; then + if ! _get_fly_cmd &>/dev/null; then log_error "flyctl not found in PATH after installation" return 1 fi @@ -71,14 +83,8 @@ ensure_fly_cli() { # Try to get token from flyctl CLI if available _try_flyctl_auth() { - local fly_cmd="" - if command -v fly &>/dev/null; then - fly_cmd="fly" - elif command -v flyctl &>/dev/null; then - fly_cmd="flyctl" - else - return 1 - fi + local fly_cmd + fly_cmd=$(_get_fly_cmd) || return 1 local token token=$("$fly_cmd" auth token 2>/dev/null || true) @@ -94,10 +100,8 @@ _validate_fly_token() { local response response=$(fly_api GET "/apps?org_slug=personal") if echo "$response" | grep -q '"error"'; then - local error_msg - error_msg=$(echo "$response" | python3 -c "import json,sys; d=json.loads(sys.stdin.read()); print(d.get('error','No details available'))" 2>/dev/null || echo "Unable to parse error") log_error "Authentication failed: Invalid Fly.io API token" - log_error "API Error: $error_msg" + log_error "API Error: $(echo "$response" | _fly_parse_error "No details available")" log_warn "Remediation steps:" log_warn " 1. Run: fly tokens deploy" log_warn " 2. Or generate a token at: https://fly.io/dashboard" @@ -174,12 +178,12 @@ _fly_create_app() { # SECURITY: Use json_escape to prevent JSON injection local app_body app_body=$(printf '{"app_name":%s,"org_slug":%s}' "$(json_escape "$name")" "$(json_escape "$org")") - local app_response - app_response=$(fly_api POST "/apps" "$app_body") + local response + response=$(fly_api POST "/apps" "$app_body") - if echo "$app_response" | grep -q '"error"'; then + if echo "$response" | grep -q '"error"'; then local error_msg - error_msg=$(echo "$app_response" | python3 -c "import json,sys; d=json.loads(sys.stdin.read()); print(d.get('error','Unknown error'))" 2>/dev/null || echo "$app_response") + error_msg=$(echo "$response" | _fly_parse_error) if echo "$error_msg" | grep -qi "already exists"; then log_info "App '$name' already exists, reusing it" return 0 @@ -196,18 +200,11 @@ _fly_create_app() { log_info "App '$name' created" } -# Create a Fly.io machine via the Machines API -# Sets FLY_MACHINE_ID and FLY_APP_NAME on success -_fly_create_machine() { - local name="$1" - local region="$2" - local vm_memory="$3" - - log_step "Creating Fly.io machine (region: $region, memory: ${vm_memory}MB)..." - - # SECURITY: Pass values via environment variables to prevent Python injection - local machine_body - machine_body=$(_FLY_NAME="$name" _FLY_REGION="$region" _FLY_MEM="$vm_memory" python3 -c " +# Build JSON request body for Fly.io machine creation +# SECURITY: Pass values via environment variables to prevent Python injection +_fly_build_machine_body() { + local name="$1" region="$2" vm_memory="$3" + _FLY_NAME="$name" _FLY_REGION="$region" _FLY_MEM="$vm_memory" python3 -c " import json, os body = { 'name': os.environ['_FLY_NAME'], @@ -226,16 +223,27 @@ body = { } } print(json.dumps(body)) -") +" +} + +# Create a Fly.io machine via the Machines API +# Sets FLY_MACHINE_ID and FLY_APP_NAME on success +_fly_create_machine() { + local name="$1" + local region="$2" + local vm_memory="$3" + + log_step "Creating Fly.io machine (region: $region, memory: ${vm_memory}MB)..." + + local machine_body + machine_body=$(_fly_build_machine_body "$name" "$region" "$vm_memory") local response response=$(fly_api POST "/apps/$name/machines" "$machine_body") if echo "$response" | grep -q '"error"'; then log_error "Failed to create Fly.io machine" - local error_msg - error_msg=$(echo "$response" | python3 -c "import json,sys; d=json.loads(sys.stdin.read()); print(d.get('error','Unknown error'))" 2>/dev/null || echo "$response") - log_error "API Error: $error_msg" + log_error "API Error: $(echo "$response" | _fly_parse_error)" log_warn "Common issues:" log_warn " - Insufficient account balance or payment method required" log_warn " - Region unavailable (try different FLY_REGION)" @@ -244,7 +252,7 @@ print(json.dumps(body)) return 1 fi - FLY_MACHINE_ID=$(echo "$response" | python3 -c "import json,sys; print(json.loads(sys.stdin.read())['id'])") + FLY_MACHINE_ID=$(_extract_json_field "$response" "d['id']") export FLY_MACHINE_ID FLY_APP_NAME="$name" log_info "Machine created: ID=$FLY_MACHINE_ID, App=$name" } @@ -259,10 +267,8 @@ _fly_wait_for_machine_start() { log_step "Waiting for machine to start..." while [[ "$attempt" -le "$max_attempts" ]]; do - local status_response - status_response=$(fly_api GET "/apps/$name/machines/$machine_id") local state - state=$(echo "$status_response" | python3 -c "import json,sys; print(json.loads(sys.stdin.read()).get('state','unknown'))") + state=$(_extract_json_field "$(fly_api GET "/apps/$name/machines/$machine_id")" "d.get('state','unknown')") if [[ "$state" == "started" ]]; then log_info "Machine is running" @@ -311,9 +317,7 @@ run_server() { # SECURITY: Properly escape command to prevent injection local escaped_cmd escaped_cmd=$(printf '%q' "$cmd") - local fly_cmd="fly" - command -v fly &>/dev/null || fly_cmd="flyctl" - "$fly_cmd" ssh console -a "$FLY_APP_NAME" -C "bash -c $escaped_cmd" --quiet 2>/dev/null + "$(_get_fly_cmd)" ssh console -a "$FLY_APP_NAME" -C "bash -c $escaped_cmd" --quiet 2>/dev/null } # Upload a file to the machine via base64 encoding through exec @@ -340,9 +344,7 @@ interactive_session() { # SECURITY: Properly escape command to prevent injection local escaped_cmd escaped_cmd=$(printf '%q' "$cmd") - local fly_cmd="fly" - command -v fly &>/dev/null || fly_cmd="flyctl" - "$fly_cmd" ssh console -a "$FLY_APP_NAME" -C "bash -c $escaped_cmd" + "$(_get_fly_cmd)" ssh console -a "$FLY_APP_NAME" -C "bash -c $escaped_cmd" } # Destroy a Fly.io machine and app @@ -352,8 +354,10 @@ destroy_server() { log_step "Destroying Fly.io app and machines for '$app_name'..." # List and destroy all machines in the app - local machines=$(fly_api GET "/apps/$app_name/machines") - local machine_ids=$(echo "$machines" | python3 -c " + local machines + machines=$(fly_api GET "/apps/$app_name/machines") + local machine_ids + machine_ids=$(echo "$machines" | python3 -c " import json, sys data = json.loads(sys.stdin.read()) if isinstance(data, list): diff --git a/netcup/lib/common.sh b/netcup/lib/common.sh index f4cfafec..cbc33e01 100644 --- a/netcup/lib/common.sh +++ b/netcup/lib/common.sh @@ -23,6 +23,28 @@ fi readonly NETCUP_API_BASE="https://ccp.netcup.net/run/webservice/servers/endpoint.php" # SSH_OPTS is now defined in shared/common.sh +# Check if a Netcup API response indicates success +# Returns 0 on success, 1 on failure +_netcup_is_success() { + local response="$1" + _extract_json_field "$response" "d.get('status','')" | grep -q "^success$" +} + +# Build JSON login request body +_netcup_build_login_body() { + python3 -c " +import json, sys +print(json.dumps({ + 'action': 'login', + 'param': { + 'customernumber': sys.argv[1], + 'apikey': sys.argv[2], + 'apipassword': sys.argv[3] + } +})) +" "$1" "$2" "$3" +} + # Netcup uses session-based authentication with API credentials # Get session token from API credentials netcup_get_session() { @@ -36,17 +58,7 @@ netcup_get_session() { fi local body - body=$(python3 -c " -import json, sys -print(json.dumps({ - 'action': 'login', - 'param': { - 'customernumber': sys.argv[1], - 'apikey': sys.argv[2], - 'apipassword': sys.argv[3] - } -})) -" "$customer_number" "$api_key" "$api_password") + body=$(_netcup_build_login_body "$customer_number" "$api_key" "$api_password") local response response=$(curl -fsSL -X POST "$NETCUP_API_BASE" \ @@ -56,25 +68,13 @@ print(json.dumps({ return 1 } - # Extract session ID (apisessionid) - local session_id - session_id=$(echo "$response" | python3 -c " -import json, sys -try: - data = json.loads(sys.stdin.read()) - if data.get('status') == 'success': - print(data['responsedata']['apisessionid']) - else: - sys.exit(1) -except: - sys.exit(1) -" 2>/dev/null) || { + if ! _netcup_is_success "$response"; then log_error "Failed to authenticate with Netcup API" log_error "Response: $response" return 1 - } + fi - echo "$session_id" + _extract_json_field "$response" "d['responsedata']['apisessionid']" } # Centralized API call wrapper for Netcup @@ -217,21 +217,14 @@ print(json.dumps(param)) # Sets NETCUP_SERVER_IP on success _netcup_wait_for_ip() { log_step "Waiting for IP assignment..." - local ip="" - local attempts=0 + local ip="" attempts=0 while [[ -z "$ip" ]] && [[ $attempts -lt 60 ]]; do sleep 5 local info_response info_response=$(netcup_api "getVServerInfo" "{\"vserverid\": \"$NETCUP_SERVER_ID\"}") - ip=$(echo "$info_response" | python3 -c " -import json, sys -try: - data = json.loads(sys.stdin.read()) - if data.get('status') == 'success': - print(data['responsedata'].get('ipv4', '')) -except: - pass -" 2>/dev/null || echo "") + if _netcup_is_success "$info_response"; then + ip=$(_extract_json_field "$info_response" "d.get('responsedata',{}).get('ipv4','')") + fi attempts=$((attempts + 1)) if [[ -z "$ip" ]] && [[ $((attempts % 5)) -eq 0 ]]; then log_step "Still waiting for IP assignment... (attempt ${attempts}/60)" @@ -270,15 +263,9 @@ create_server() { local response response=$(netcup_api "createVServer" "$param") - # Check for errors - local status - status=$(echo "$response" | python3 -c "import json, sys; print(json.loads(sys.stdin.read()).get('status', 'error'))") - - if [[ "$status" != "success" ]]; then + if ! _netcup_is_success "$response"; then log_error "Failed to create Netcup VPS" - local error_msg - error_msg=$(echo "$response" | python3 -c "import json,sys; print(json.loads(sys.stdin.read()).get('longmessage','Unknown error'))" 2>/dev/null || echo "$response") - log_error "API Error: $error_msg" + log_error "API Error: $(_extract_json_field "$response" "d.get('longmessage','Unknown error')")" log_error "" log_error "Common issues:" log_error " - Insufficient account balance" @@ -290,7 +277,7 @@ create_server() { fi # Extract server ID - NETCUP_SERVER_ID=$(echo "$response" | python3 -c "import json,sys; print(json.loads(sys.stdin.read())['responsedata']['vserverid'])") + NETCUP_SERVER_ID=$(_extract_json_field "$response" "d['responsedata']['vserverid']") export NETCUP_SERVER_ID log_info "VPS created: ID=$NETCUP_SERVER_ID" @@ -313,10 +300,7 @@ destroy_server() { local response response=$(netcup_api "deleteVServer" "{\"vserverid\": \"$server_id\"}") - local status - status=$(echo "$response" | python3 -c "import json, sys; print(json.loads(sys.stdin.read()).get('status', 'error'))") - - if [[ "$status" != "success" ]]; then + if ! _netcup_is_success "$response"; then log_error "Failed to destroy VPS: $response" return 1 fi