Skip to content

Commit 1dac4ff

Browse files
dholtclaude
andcommitted
fix: address Copilot review feedback on MAAS deploy and inventory
- Move API key validation from maas_auth_header() to load_config() so exit works properly (exit in command substitution only kills subshell) - Accept MAAS_SSH_BASTION env var (consistent with inventory script) and convert to ProxyCommand; MAAS_SSH_PROXY still works as direct override - Quote ssh_bastion value in proxy command to handle spaces/special chars - Use os.environ instead of shell interpolation for network_filter in get_ip() to prevent potential code injection - Deduplicate hosts in inventory when machine has both old and aliased tags (e.g., both kube-master and kube_control_plane) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Douglas Holt <dholt@nvidia.com>
1 parent 97051de commit 1dac4ff

File tree

2 files changed

+20
-6
lines changed

2 files changed

+20
-6
lines changed

scripts/maas_deploy.sh

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,18 @@ load_config() {
6969
api_url) [[ -z "${MAAS_API_URL:-}" ]] && MAAS_API_URL="$value" ;;
7070
api_key) [[ -z "${MAAS_API_KEY:-}" ]] && MAAS_API_KEY="$value" ;;
7171
ssh_user) [[ -z "${MAAS_SSH_USER:-}" ]] && MAAS_SSH_USER="$value" ;;
72-
ssh_bastion) [[ -z "${MAAS_SSH_PROXY:-}" ]] && MAAS_SSH_PROXY="ssh -W %h:%p -q ${value}" ;;
72+
ssh_bastion) [[ -z "${MAAS_SSH_PROXY:-}" && -z "${MAAS_SSH_BASTION:-}" ]] && MAAS_SSH_BASTION="$value" ;;
7373
network) [[ -z "${MAAS_NETWORK:-}" ]] && MAAS_NETWORK="$value" ;;
7474
machines) [[ -z "${MAAS_MACHINES:-}" ]] && MAAS_MACHINES="$value" ;;
7575
esac
7676
done < "$config_file"
7777
fi
7878

79+
# Build SSH proxy from MAAS_SSH_BASTION if MAAS_SSH_PROXY not set directly
80+
if [[ -z "${MAAS_SSH_PROXY:-}" && -n "${MAAS_SSH_BASTION:-}" ]]; then
81+
MAAS_SSH_PROXY="ssh -W %h:%p -q $(printf %q "${MAAS_SSH_BASTION}")"
82+
fi
83+
7984
# Defaults for anything still unset
8085
MAAS_API_URL="${MAAS_API_URL:-}"
8186
MAAS_API_KEY="${MAAS_API_KEY:-}"
@@ -95,6 +100,14 @@ load_config() {
95100
echo "Set it in config/maas-inventory.yml or as an environment variable"
96101
exit 1
97102
fi
103+
104+
# Validate API key format (must be consumer_key:token_key:token_secret)
105+
local _ck _tk _ts _extra
106+
IFS=':' read -r _ck _tk _ts _extra <<< "$MAAS_API_KEY"
107+
if [[ -n "${_extra:-}" || -z "${_ck}" || -z "${_tk}" || -z "${_ts}" ]]; then
108+
echo "ERROR: MAAS_API_KEY must be in format consumer_key:token_key:token_secret"
109+
exit 1
110+
fi
98111
}
99112

100113
# --- Argument Parsing ---------------------------------------------------------
@@ -149,6 +162,7 @@ parse_args() {
149162
# --- MAAS API Helpers ---------------------------------------------------------
150163

151164
maas_auth_header() {
165+
# API key format already validated in load_config()
152166
local consumer_key token_key token_secret
153167
IFS=':' read -r consumer_key token_key token_secret <<< "$MAAS_API_KEY"
154168
local nonce timestamp
@@ -191,11 +205,10 @@ print(m['status'])
191205

192206
get_ip() {
193207
local system_id="$1"
194-
local network_filter="${MAAS_NETWORK:-}"
195-
maas_get "/machines/${system_id}/" | python3 -c "
196-
import json, sys
208+
maas_get "/machines/${system_id}/" | MAAS_NETWORK="${MAAS_NETWORK:-}" python3 -c "
209+
import json, os, sys
197210
m = json.load(sys.stdin)
198-
network = '${network_filter}'
211+
network = os.environ.get('MAAS_NETWORK', '')
199212
for iface in m.get('interface_set', []):
200213
for link in iface.get('links', []):
201214
ip = link.get('ip_address', '')

scripts/maas_inventory.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,8 @@ def build_inventory(config):
242242
inventory[group] = {"hosts": [], "vars": {}}
243243
elif "hosts" not in inventory[group]:
244244
inventory[group]["hosts"] = []
245-
inventory[group]["hosts"].append(hostname)
245+
if hostname not in inventory[group]["hosts"]:
246+
inventory[group]["hosts"].append(hostname)
246247

247248
return inventory
248249

0 commit comments

Comments
 (0)