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>

Author: dholt

scripts/maas_deploy.sh

@@ -15,7 +15,8 @@
 # Configuration:
 #   Reads config/maas-inventory.yml (same config as maas_inventory.py).
 #   Environment variables override config file values:
-#     MAAS_API_URL, MAAS_API_KEY, MAAS_MACHINES, MAAS_SSH_USER, MAAS_SSH_PROXY
+#     MAAS_API_URL, MAAS_API_KEY, MAAS_MACHINES, MAAS_SSH_USER,
+#     MAAS_SSH_BASTION (or MAAS_SSH_PROXY for full ProxyCommand override)
 #
 # Examples:
 #   ./scripts/maas_deploy.sh --os noble --profile k8s
@@ -69,13 +70,18 @@ load_config() {
                 api_url)     [[ -z "${MAAS_API_URL:-}" ]]  && MAAS_API_URL="$value" ;;
                 api_key)     [[ -z "${MAAS_API_KEY:-}" ]]   && MAAS_API_KEY="$value" ;;
                 ssh_user)    [[ -z "${MAAS_SSH_USER:-}" ]]  && MAAS_SSH_USER="$value" ;;
-                ssh_bastion) [[ -z "${MAAS_SSH_PROXY:-}" ]] && MAAS_SSH_PROXY="ssh -W %h:%p -q ${value}" ;;
+                ssh_bastion) [[ -z "${MAAS_SSH_PROXY:-}" && -z "${MAAS_SSH_BASTION:-}" ]] && MAAS_SSH_BASTION="$value" ;;
                 network)     [[ -z "${MAAS_NETWORK:-}" ]]   && MAAS_NETWORK="$value" ;;
                 machines)    [[ -z "${MAAS_MACHINES:-}" ]]  && MAAS_MACHINES="$value" ;;
             esac
         done < "$config_file"
     fi
 
+    # Build SSH proxy from MAAS_SSH_BASTION if MAAS_SSH_PROXY not set directly
+    if [[ -z "${MAAS_SSH_PROXY:-}" && -n "${MAAS_SSH_BASTION:-}" ]]; then
+        MAAS_SSH_PROXY="ssh -W %h:%p -q ${MAAS_SSH_BASTION}"
+    fi
+
     # Defaults for anything still unset
     MAAS_API_URL="${MAAS_API_URL:-}"
     MAAS_API_KEY="${MAAS_API_KEY:-}"
@@ -95,6 +101,12 @@ load_config() {
         echo "Set it in config/maas-inventory.yml or as an environment variable"
         exit 1
     fi
+
+    # Validate API key format: exactly 3 non-empty colon-separated parts
+    if [[ ! "$MAAS_API_KEY" =~ ^[^:]+:[^:]+:[^:]+$ ]]; then
+        echo "ERROR: MAAS_API_KEY must be in format consumer_key:token_key:token_secret"
+        exit 1
+    fi
 }
 
 # --- Argument Parsing ---------------------------------------------------------
@@ -149,6 +161,7 @@ parse_args() {
 # --- MAAS API Helpers ---------------------------------------------------------
 
 maas_auth_header() {
+    # API key format already validated in load_config()
     local consumer_key token_key token_secret
     IFS=':' read -r consumer_key token_key token_secret <<< "$MAAS_API_KEY"
     local nonce timestamp
@@ -191,11 +204,10 @@ print(m['status'])
 
 get_ip() {
     local system_id="$1"
-    local network_filter="${MAAS_NETWORK:-}"
-    maas_get "/machines/${system_id}/" | python3 -c "
-import json, sys
+    maas_get "/machines/${system_id}/" | MAAS_NETWORK="${MAAS_NETWORK:-}" python3 -c "
+import json, os, sys
 m = json.load(sys.stdin)
-network = '${network_filter}'
+network = os.environ.get('MAAS_NETWORK', '')
 for iface in m.get('interface_set', []):
     for link in iface.get('links', []):
         ip = link.get('ip_address', '')

scripts/maas_inventory.py

@@ -193,6 +193,9 @@ def build_inventory(config):
     for parent, children in GROUP_CHILDREN.items():
         inventory[parent] = {"children": children, "hosts": []}
 
+    # Track group membership with sets to avoid O(n^2) dedup
+    group_members = {}
+
     for machine in machines:
         # Only include Deployed machines
         if machine.get("status") != 6:
@@ -242,7 +245,11 @@ def build_inventory(config):
                 inventory[group] = {"hosts": [], "vars": {}}
             elif "hosts" not in inventory[group]:
                 inventory[group]["hosts"] = []
-            inventory[group]["hosts"].append(hostname)
+            if group not in group_members:
+                group_members[group] = set()
+            if hostname not in group_members[group]:
+                group_members[group].add(hostname)
+                inventory[group]["hosts"].append(hostname)
 
     return inventory