Skip to content

fix(pxe): use default HTTP port for embedded iPXE#3001

Merged
osu merged 1 commit into
NVIDIA:mainfrom
osu:fix/2598-ipxe-http-port
Jun 30, 2026
Merged

fix(pxe): use default HTTP port for embedded iPXE#3001
osu merged 1 commit into
NVIDIA:mainfrom
osu:fix/2598-ipxe-http-port

Conversation

@osu

@osu osu commented Jun 29, 2026

Copy link
Copy Markdown
Member

Description

Remove the embedded iPXE script hardcoded :8080 suffix so its whoami and boot chains use the standard HTTP port at the DHCP-provided next-server address.

This change:

  • keeps the DHCP next-server IPv4 address as the PXE endpoint;
  • routes both embedded iPXE chains through standard HTTP port 80, matching the external PXE service; and
  • adds a regression test for the rendered static iPXE menu.

Related issues

Closes #2598

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Passed locally:

  • cargo test -p carbide-ipxe-renderer --locked (35 tests)
  • cargo clippy -p carbide-ipxe-renderer --all-targets --locked -- -D warnings
  • rustup run nightly-2026-06-16 rustfmt --check crates/ipxe-renderer/src/lib.rs
  • git diff --check

Additional Notes

The embedded iPXE script now relies on HTTP default port 80, which is exposed by nico-pxe-external-80 and targets the PXE container on port 8080.

I also attempted full EFI artifact builds locally. The native ARM build parsed the embedded script but then hit existing TPM-patch warnings treated as errors; the AMD64 Docker build reached compilation but hit a GCC internal compiler error under emulation. Neither failure references this change; the repository CI boot-artifact build remains the authoritative validation for those binaries.

Signed-off-by: Hasan Khan <hasank@nvidia.com>
@osu osu requested a review from a team as a code owner June 29, 2026 23:43
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0489cd1e-8f94-413d-a515-91f663f8a225

📥 Commits

Reviewing files that changed from the base of the PR and between 278b144 and 9839388.

📒 Files selected for processing (2)
  • crates/ipxe-renderer/src/lib.rs
  • pxe/ipxe/local/embed.ipxe

Summary by CodeRabbit

  • Bug Fixes
    • Updated PXE boot links to use the DHCP-provided server address on the standard HTTP port, avoiding references to an internal container port.
  • Tests
    • Added coverage to ensure the embedded iPXE menu uses the expected HTTP endpoint for key boot requests.

Walkthrough

Removes the hardcoded :8080 port from the embedded iPXE script's chain URLs for the /api/v0/pxe/whoami and /api/v0/pxe/boot endpoints, replacing them with standard HTTP on ${next-server}. A corresponding unit test validates the template no longer contains ${next-server}:8080.

Changes

iPXE Port Hardcoding Fix

Layer / File(s) Summary
Remove :8080 and add regression test
pxe/ipxe/local/embed.ipxe, crates/ipxe-renderer/src/lib.rs
Chain URLs for :whoami and :nico drop the explicit :8080 port in favor of standard HTTP on ${next-server}. Inline comments document the DHCP next-server semantics. A new unit test asserts STATIC_IPXE_MENU_TEMPLATE contains correct ${next-server}-based URLs and does not contain ${next-server}:8080.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main fix: using the default HTTP port for embedded iPXE.
Description check ✅ Passed The description matches the change set and accurately explains the port and test updates.
Linked Issues check ✅ Passed The PR addresses #2598 by removing the hardcoded 8080 port and using the DHCP-provided next-server with the default HTTP port.
Out of Scope Changes check ✅ Passed The changes stay focused on the PXE URL fix and its regression test, with no unrelated additions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@osu osu merged commit da6aedc into NVIDIA:main Jun 30, 2026
56 checks passed
@osu osu deleted the fix/2598-ipxe-http-port branch June 30, 2026 00:51
@github-actions

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
boot-artifacts-aarch64 3 0 0 3 0 0
boot-artifacts-x86_64 3 0 0 3 0 0
forge-admin-cli-x86_64 288 6 26 105 7 144
machine-validation-runner 751 30 190 274 36 221
machine_validation 751 30 190 274 36 221
machine_validation-aarch64 751 30 190 274 36 221
nvmetal-carbide 751 30 190 274 36 221
TOTAL 3298 126 786 1207 151 1028

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Embedded iPXE hardcodes port 8080

2 participants