Add isset guards to enable runtime variable overrides#1756
Add isset guards to enable runtime variable overrides#1756Skyrant wants to merge 3 commits intonetbootxyz:developmentfrom
Conversation
Add isset guards to all set commands in boot.cfg.j2 and menu templates
to allow variables to be overridden in local-vars.ipxe without requiring
a rebuild. This makes netboot.xyz more flexible for users who want to
customize mirrors, timeouts, and menu options at boot time.
Changes:
- boot.cfg.j2: Add isset guards to all global vars, mirror loop, architecture
flags, and cloud-specific settings
- tinycore.ipxe.j2: Add isset guard to tinycore_mirror
- k3os.ipxe.j2: Add isset guard to k3os_mirror
- debian.ipxe.j2: Add isset guard to debian_mirror, remove redundant no-op
- ubuntu.ipxe.j2: Add isset guard to ubuntu_mirror
- archlinux.ipxe.j2: Add clarifying comment about archlinux_mirror
All variables now follow the pattern: isset ${var} || set var value
This is fully backwards compatible and follows existing patterns used by
live_endpoint and boot_timeout variables.
Code Review SummaryStatus: No New Issues Found | Recommendation: Address Existing Comments OverviewThis review found no new issues beyond the 7 existing inline comments already posted. The changes consistently apply Key Observations✅ Consistent Pattern: All changes follow the idiomatic iPXE pattern ✅ Backwards Compatible: When variables are not pre-set, defaults are applied as before ✅ Debian Template Improvement: The removal of the redundant
Files Reviewed (2 files)
|
There was a problem hiding this comment.
Pull request overview
This PR updates several netboot.xyz iPXE menu templates to make key variables (mirrors, global config, and some boot/menu flags) “override-friendly” by only setting defaults when variables are not already set.
Changes:
- Add
isset ... || set ...guards for various mirrors and global variables to preserve externally-provided overrides. - Adjust Debian/Ubuntu older-release handling and add clarifying comments about where mirrors are set.
- Add a small clarifying comment in the Arch Linux menu.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| roles/netbootxyz/templates/menu/boot.cfg.j2 | Adds isset guards for global vars, per-release mirror/base_dir vars, arch/platform menu flags, and cloud/provider specifics. |
| roles/netbootxyz/templates/menu/debian.ipxe.j2 | Uses isset guard when setting archive mirror for older releases; removes a self-assignment in :mirrorcfg and replaces with a comment. |
| roles/netbootxyz/templates/menu/ubuntu.ipxe.j2 | Uses isset guard when setting archive mirror for older releases. |
| roles/netbootxyz/templates/menu/tinycore.ipxe.j2 | Makes tinycore_mirror default-set only when unset. |
| roles/netbootxyz/templates/menu/k3os.ipxe.j2 | Makes k3os_mirror default-set only when unset. |
| roles/netbootxyz/templates/menu/archlinux.ipxe.j2 | Adds a comment about archlinux_mirror coming from boot.cfg.j2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use string concatenation to properly construct iPXE variable references
like ${ubuntu_mirror} instead of invalid ${{{ key }}_mirror} syntax.
Remove isset guards from architecture/platform-specific settings that must be enforced based on hardware detection: - ipxe_disk: Bootloader must match detected platform - menu_* flags: Menu options must respect arch/platform capabilities Remove isset guards from distro-specific mirror variables not in the boot.cfg mirror loop (debian, ubuntu, k3os, tinycore). Keep isset guards only for truly user-configurable settings: - Global config (site_name, boot_domain, timeouts, etc.) - Mirror loop variables for distros in releases dict - Cloud-specific boot parameters (cmdline) This prevents users from accidentally enabling incompatible menus or bootloaders while still allowing customization of mirrors and timeouts.
|
fixed the issues, was a bit over-zealous with the isset changes. |
Add isset guards to all iPXE variable assignments
Summary
This PR adds
issetguards to allsetcommands across iPXE menu templates to enable runtime variable overrides vialocal-vars.ipxewithout requiring a full rebuild. This change makes netboot.xyz more flexible for users who want to customize behavior (mirrors, timeouts, menu options) at boot time.Motivation
Currently, most variables in boot.cfg.j2 and menu templates are set unconditionally, which means they cannot be overridden in local-vars.ipxe at runtime. Only a few variables (like
live_endpointandboot_timeout) already hadissetguards. This inconsistency creates limitations for users who:Changes Made
boot.cfg.j2
Added
issetguards to all variable assignments:site_name,boot_domain,memdisk,sigs_enabled,sigs,ipxe_disk${distro}_mirrorand${distro}_base_dirvariablesmenu_*variables across architecture sections (:architectures,:x86_64,:i386,:arm64,:efi)cmdline,ipxe_disk, and menu overrides in:gce,:metal_x86_64,:metal_arm64sectionsIndividual Menu Templates
issetguard totinycore_mirrorissetguard tok3os_mirrorissetguard todebian_mirrorin:older_releasesectionset debian_mirror ${debian_mirror}with explanatory commentissetguard toubuntu_mirrorin:older_releasesectionarchlinux_mirroris already handled by boot.cfg.j2Pattern Used
All changes follow the same idiomatic iPXE pattern:
This ensures that if a variable is already set (e.g., in local-vars.ipxe which loads first), it won't be overwritten.
Boot Flow and Variable Loading
Problem: Without isset Guards
sequenceDiagram participant TFTP as TFTP Server participant LV as local-vars.ipxe participant BC as boot.cfg.j2 participant Menu as Menu System TFTP->>LV: 1. Chain load Note over LV: set fedora_mirror http://my-mirror.lan Note over LV: ✓ Variable set by user LV->>BC: 2. Chain load Note over BC: set fedora_mirror {{ releases.fedora.mirror }} Note over BC: ❌ OVERWRITES user's mirror! BC->>Menu: 3. Boot menu Note over Menu: Uses default mirror<br/>User override ignored 😞Solution: With isset Guards
sequenceDiagram participant TFTP as TFTP Server participant LV as local-vars.ipxe participant BC as boot.cfg.j2 participant Menu as Menu System TFTP->>LV: 1. Chain load Note over LV: set fedora_mirror http://my-mirror.lan Note over LV: ✓ Variable set by user LV->>BC: 2. Chain load Note over BC: isset ${fedora_mirror} ‖ set fedora_mirror ... Note over BC: ✓ Checks first, doesn't overwrite! BC->>Menu: 3. Boot menu Note over Menu: Uses user's mirror<br/>Override respected! 🎉Behavior
Before
Variables set in boot.cfg.j2 and menu templates would always overwrite any pre-existing values, making local-vars.ipxe overrides ineffective for most variables.
After
Users can now pre-set variables in local-vars.ipxe, and they will be respected throughout the boot process. For example:
Testing
ansible-playbook site.yml --syntax-checkBackwards Compatibility
Fully backwards compatible. When variables are not pre-set, they fall back to the same defaults as before. Existing deployments will see no change in behavior unless they explicitly leverage the new override capability.
Related
This addresses the inconsistency mentioned in local-vars.ipxe.j2 comments, where variables like
rhel_base_urlandwin_base_urlwere documented as overridable, but many other variables were not.Files Changed
roles/netbootxyz/templates/menu/boot.cfg.j2(108 lines changed)roles/netbootxyz/templates/menu/archlinux.ipxe.j2(1 line added - comment)roles/netbootxyz/templates/menu/debian.ipxe.j2(4 lines changed)roles/netbootxyz/templates/menu/k3os.ipxe.j2(2 lines changed)roles/netbootxyz/templates/menu/tinycore.ipxe.j2(2 lines changed)roles/netbootxyz/templates/menu/ubuntu.ipxe.j2(2 lines changed)Total: 60 insertions(+), 59 deletions(-)