deploy-gns3-server/TODO.md

TODO.md Convention Adoption

[x] done, [-] partial, [ ] not started

---

Phase 1: Low-Hanging Fruit (done or nearly done)

• set -euo pipefail + IFS=$'\n\t'
• readonly PATH (Linux-only, homebrew paths removed)
• Structured header block with metadata
• _READONLY_GLOBAL_ naming for constants
• _local_var naming for function locals and iterators
• "${VAR}" expansion everywhere (quoted + braced)
• log_* hierarchy (info, ok, warn, error, fatal)
• log_warn_sticky->WARN_MESSAGES array
• ensure_directory / ensure_config / require_file helpers
• fetch_file (stdout-based, composable)
• render_template (stdin-based, {{KEY}} substitution)
• enable_and_start with retry loop
• Predicates: file_exists, dir_exists, var_exists, port_open
• if/then/fi instead of [[ ]] && shorthand (most converted)
• encrypted_copy uses openssl enc with -pbkdf2
• Decrypt commands match encryption params
• gns3_repo_present handles deb822 format
• Sudoers lines have no leading whitespace
• Stray box-drawing chars removed from section headers
• fetch_resource removed fetch_file used everywhere
• IP lookups cached as SERVER_PUBLIC_IP / SERVER_PRIVATE_IP / SERVER_HOSTNAME
• No redundant IP re-checks mid-script
• .shellcheckrc with justified exclusions
• [-] Remaining [[ ]] && / || patterns most converted, grep to find stragglers

---

Phase 2: Structural Alignment

• Replace on_error() + separate INT/TERM trap with single cleanup() on EXIT
  • Convention: trap cleanup EXIT as first line of main()
  • cleanup() preserves exit code, disarms traps, set +e
  • Restart GNS3 on failure (current on_error behavior) moves into cleanup
  • INT/TERM: set context flag and exit - cleanup runs via EXIT trap
• Add setup_tempdir() - centralized TEMP_DIR used by apt_retry and ensure_config
  • Currently both use ad-hoc mktemp
  • ensure_config should use mktemp "${TEMP_DIR}/..." instead of mktemp "${_path}.XXXXXX"
• Add tracking arrays wired to helpers:
  • DIRECTORIES_CREATED ← ensure_directory appends
  • SERVICES_ENABLED ← enable_and_start appends
  • PACKAGES_INSTALLED ← install_packages populates (optional)
  • Summary reads from these arrays
• Add ROLLBACK_SERVICES / ROLLBACK_DIRS typed registries
  • cleanup() iterates in reverse on failure
• Replace flock lockfile with mkdir-based lock + stale PID detection (skeleton pattern)
• Remove WARNINGS_OCCURRED or use it - currently set but never read
• Add require_args to helper functions (from skeleton)

---

Phase 3: Naming & Organization

• Standardize template placeholder names
  • Current: mixed _DEPLOY_MARKER_ / _listen_host / GNS3_PORT / _hw_accel
  • Target: consistent SCREAMING_SNAKE for all placeholders (they're config values, not locals)
• Rename phase functions to convention verbs:
  • preflight_checks->preflight
  • setup_config_server->config_server
  • configure_firewall->firewall
  • apply_sysctl_hardening->hardening
  • start_services->(merge into per-service phases or remove)
  • print_summary->summary
• Rename sub-functions with phase prefix:
  • setup_groups->acct_create_groups
  • setup_gns3_user->acct_create_gns3_user
  • propagate_groups_to_invoker->acct_add_invoker
  • add_gns3_repository->repo_add_gns3
  • add_docker_repository->repo_add_docker
  • VPN functions: ovpn_* and wg_* prefixes
• Reorder function definitions to match execution order
• Add INDEX section to header mapping functions to sections
• Add VERSION constant, print in --help and summary
• Cache _SCRIPT_DIR_ as readonly global (currently computed multiple times in fetch_file)

---

Phase 4: Macro Refactor

• Two-tier main() structure:

  main->bootstrap->preflight->repositories->packages →
         accounts->gns3->docker->openvpn->wireguard →
         welcome->firewall->hardening->config_server →
         validate->summary
  

• Each phase is a noun function containing only sub-function calls
• Sub-functions use phase_verb_noun naming
• Phase banner comments with purpose description
• Flag gating in main() with if/then:

  if [[ "${WITH_DOCKER}" -eq 1 ]]; then docker; fi
  if [[ "${WITH_OPENVPN}" -eq 1 ]]; then openvpn; fi
  

• Add predicate functions for recurring checks:
  • service_active / user_exists / interface_up / repo_present / packages_installed
• Move apply_option_flags (rollup) into its own named phase or call from main() before bootstrap

---

Phase 5: Multi-File Split (future, when script exceeds ~1000 LOC of logic)

• conf/ - readonly vars/arrays per feature
• lib/ source-only helpers, no main()
• mod/ executable modules with numeric prefix ordering
• bootstrap.sh top-level orchestrator, executes mod/*.sh
• Source guards: [[ -n "${_LIB_X_LOADED:-}" ]] && return 0
• Direct-execution guards on sourced files

---

Ongoing / Per-Commit

• shfmt -i 2 -ci before commit
• shellcheck zero warnings (with .shellcheckrc exclusions)
• Update Changed: date in header on substantive edits
• Update INDEX when function structure changes
• Convention: Check / Do / Verify for every action
• Convention: secrets never as function arguments; use env/files/stdin
• Convention: backup_file before modifying existing configs (not yet implemented)

---

Known Deferred Items

• check_integrity() SHA256 validation for downloaded resources
• --with-optimization phase TCP/KVM tuning (BBR, swappiness, etc.)
• 26.04 codename fallback check GNS3 PPA Packages.xz for actual content
• OpenVPN client.ovpn still uses heredoc (PEM blocks don't work with render_template)
• docker.list still uses heredoc (inline $(dpkg --print-architecture) call)
• Validate function could check ufw status numbered rule count matches expectations
• Landing page: version-matched GNS3 client download URLs via JS
• GPG-signed bootstrap at lrk.cx/gns3

---