zedUpload/sftp: accept a scheme-qualified host as well as bare host:port - #90
Merged
Merged
Conversation
christoph-zededa
approved these changes
Aug 6, 2026
Contributor
|
Test failure fixed in #91 |
eriknordmark
requested changes
Aug 7, 2026
eriknordmark
left a comment
eriknordmark
left a comment
Contributor
There was a problem hiding this comment.
Please rebase once #91 is merged to we can see CI green.
getSftpClient assumed host was always a bare "host:port" and extracted the hostname for its pre-flight DNS check via a naive strings.Split(host, ":"), taking the first element. Since lf-edge/eve#5588, EVE's datastore Fqdn is always scheme-qualified for every datastore type, including SFTP (e.g. "sftp://host:port"), which broke that assumption: the first element became the literal scheme "sftp" instead of the real host, causing the SFTP download to fail with: sftpclient failed for sftp://244.244.244.244:22: lookup sftp on 127.0.0.1:53: no such host (i.e. trying to resolve "sftp") Detect which form was given via url.Parse: a real scheme-qualified authority-form URL parses with a non-empty Host, whereas a bare host:port also "succeeds" but only as an opaque URI (empty Host, with the part before the port's colon landing in Scheme/Opaque instead). Use u.Host when present, and fall back to the original bare host:port otherwise, so existing callers outside EVE are unaffected. Also replaces the naive colon-split used for the hostname-only DNS pre-check with net.SplitHostPort, which additionally fixes handling of a bracketed IPv6 literal (e.g. "[::1]:22") in both forms. Signed-off-by: Milan Lenco <milan@zededa.com>
milan-zededa
force-pushed
the
sftp-scheme
branch
from
August 7, 2026 15:19
b001e16 to
6887449
Compare
This was referenced Aug 10, 2026
milan-zededa
added a commit
to milan-zededa/eve
that referenced
this pull request
Aug 14, 2026
Update eve-libs to include lf-edge/eve-libs#90, which fixes the SFTP datastore's getSftpClient to accept a scheme-qualified host (sftp://host:port) as well as the bare host:port form. Since EVE requires datastore Fqdn to always be scheme-qualified, SFTP downloads were failing with a DNS lookup on the literal "sftp" scheme instead of the real hostname. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Milan Lenco <milan@zededa.com>
eriknordmark
pushed a commit
to lf-edge/eve
that referenced
this pull request
Aug 14, 2026
Update eve-libs to include lf-edge/eve-libs#90, which fixes the SFTP datastore's getSftpClient to accept a scheme-qualified host (sftp://host:port) as well as the bare host:port form. Since EVE requires datastore Fqdn to always be scheme-qualified, SFTP downloads were failing with a DNS lookup on the literal "sftp" scheme instead of the real hostname. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Milan Lenco <milan@zededa.com>
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.
getSftpClientassumed host was always a barehost:portand extracted the hostname for its pre-flight DNS check via a naivestrings.Split(host, ":"), taking the first element. Since lf-edge/eve#5588, EVE's datastore Fqdn is required to be always scheme-qualified for every datastore type, including SFTP (e.g.sftp://host:port), which broke that assumption: the first element became the literal schemesftpinstead of the real host, causing the SFTP download to fail with:(i.e. trying to resolve
sftpinstead ofmy-server)Detect which form was given via
url.Parse: a real scheme-qualified authority-form URL parses with a non-empty Host, whereas a barehost:portalso "succeeds" but only as an opaque URI (empty Host, with the part before the port's colon landing in Scheme/Opaque instead). Useu.Hostwhen present, and fall back to the original barehost:portotherwise, so existing callers outside EVE are unaffected.Also replaces the naive colon-split used for the hostname-only DNS pre-check with
net.SplitHostPort, which additionally fixes handling of a bracketed IPv6 literal (e.g.[::1]:22) in both forms.In EVE, this is exercised by newly added
TestSFTPDatastore.