Repository navigation
feat(backup): support ndb through http proxy - #10544
Open
ayoub-el-kajji-v wants to merge 3 commits into
Open
ayoub-el-kajji-v wants to merge 3 commits into
ayoub-el-kajji-v wants to merge 3 commits into
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
ayoub-el-kajji-v
marked this pull request as ready for review
October 8, 2026 15:29
mpiton
requested changes
Oct 9, 2026
|
|
||
| // read the proxy response headers, the NBD server may already have sent | ||
| // some data after them | ||
| const { head, rest } = await new Promise((resolve, reject) => { |
Collaborator
There was a problem hiding this comment.
This hand-parses the proxy reply (buffering, 16 KiB cap, status line). Node's http.request({ method: 'CONNECT' }) already hands back res, socket and head on 'connect'. Could we use it and drop most of this method?
Collaborator
There was a problem hiding this comment.
I agree I think we can slim down this implementation
It can even accept a timeout parameter to ensure any failure to connect is cleared by the OS
fbeauchamp
requested changes
Oct 9, 2026
| */ | ||
| async #connectThroughHttpProxy() { | ||
| const proxy = new URL(this.#httpProxy) | ||
| const isHttps = proxy.protocol === 'https:' |
Collaborator
There was a problem hiding this comment.
a proxy protocol can also be socks5 (and probably others ) , I think we should throw an eplicit error if we have anything other than http/https , as long as we don't need it
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
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
This PR adds support for using NBD through an HTTP proxy.
Before this change, NBD could be used for continuous VM replication when the source host was directly reachable from XOA, but replication would fall back to a stream export when the source host could only be reached through an HTTP proxy. If CBT data had also been purged, this would result in a full export.
This meant that continuous replication behaved differently depending on how the source host was reached: NBD worked with direct connectivity, but was not available when the connection had to go through an HTTP proxy.
With this change, NBD connections can also be established through an HTTP proxy, allowing continuous replication to use NBD regardless of whether the source host is directly reachable or accessed through a proxy.
[XO-3028]
Tested
Lab reproducing the customer's setup:
iptables REJECTResults:
CONNECT <host>:10809tunnels (1 disk × 2 NBD connections) that carried the disk data (18 MiB); the 443 tunnels only carried XAPI calls (4.2 MiB)REJECTcounters stayed at 0can't connect through NBD,can't compute deltaorfell back to a fullwarningChecklist
Fixes #007,See xoa-support#42,See https://...)Introduced by <commit|PR>CHANGELOG.unreleased.md@xen-orchestra/lite/CHANGELOG.mdCHANGELOG.unreleased.md### Screenshotssection above this checklistReview process