machine-a-tron: Drop support for non-api PXE requests#2332
Open
kensimon wants to merge 2 commits into
Open
Conversation
Machine-a-tron has two modes for making PXE requests: Using the GetPxeInstructions gRPC API directly (use_pxe_api=true), and sending a direct HTTP request to the configured server (use_pxe_api=false). The direct HTTP request method is broken due to its reliance on the X-Forwarded-For header for conveying the client IP address, but this header has never actually worked (it's only consulted if you use the `InsecureClientIp` axum extractor, which we've never used.) The only reason use_pxe_api=false worked in the past was because we were forwarding the machine_interface_id as part of the PXE request, but that was removed in NVIDIA#1572. Rather than fixing this, we should probably just leave X-Forwarded-For unsupported anyway, since it's insecure (a client controls the headers, so they can spoof any machine's PXE script by setting the value to whatever they want) and the only thing that was trying to use it is machine-a-tron, which is only meant for mocks/testing. By always using the gRPC API for getting the PXE instructions, we lose a small amount of "realism" to the machine-a-tron mocks, by not exercising the actual HTTP request path, but it's worth it to avoid requiring X-Forwarded-For in production just to make this use case work. Our integration tests that use machine-a-tron have always used use_pxe_api=true so our actual test coverage has never included the "real" PXE HTTP request path anyway.
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
poroh
approved these changes
Jun 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Machine-a-tron has two modes for making PXE requests: Using the GetPxeInstructions gRPC API directly (use_pxe_api=true), and sending a direct HTTP request to the configured server (use_pxe_api=false).
The direct HTTP request method is broken due to its reliance on the X-Forwarded-For header for conveying the client IP address, but this header has never actually worked (it's only consulted if you use the
InsecureClientIpaxum extractor, which we've never used.) The only reason use_pxe_api=false worked in the past was because we were forwarding the machine_interface_id as part of the PXE request, but that was removed in #1572.Rather than fixing this, we should probably just leave X-Forwarded-For unsupported anyway, since it's insecure (a client controls the headers, so they can spoof any machine's PXE script by setting the value to whatever they want) and the only thing that was trying to use it is machine-a-tron, which is only meant for mocks/testing.
By always using the gRPC API for getting the PXE instructions, we lose a small amount of "realism" to the machine-a-tron mocks, by not exercising the actual HTTP request path, but it's worth it to avoid requiring X-Forwarded-For in production just to make this use case work.
Our integration tests that use machine-a-tron have always used use_pxe_api=true so our actual test coverage has never included the "real" PXE HTTP request path anyway.
Type of Change
Related Issues (Optional)
#2331
Breaking Changes
Changes are not breaking because any configs using use_pxe_api=false will now be silently ignored. machine-a-tron always supports use_pxe_api=true, so this will not break anything, only change how the mocked PXE requests are performed.
Testing
Additional Notes