Add e2e for ssh:// URLs #869
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
approved
| if u.User != nil && u.Scheme != "ssh" { | ||
| // Note that `ssh://user@host/path` URLs need to retain the user | ||
| // field. Out of caution, we only handle HTTP(S) URLs here. | ||
| if u.User != nil && (u.Scheme == "http" || u.Scheme == "https") { |
There was a problem hiding this comment.
Why not keep test on schema ssh here ?
Note that for url with the format [email protected]:kubernetes/git-sync method url.Parse return an err then it's good
err = parse "[email protected]:kubernetes/git-sync": first path segment in URL cannot contain colon
There was a problem hiding this comment.
I don't know what OTHER schemes might have the problem that ssh://... has, but I do know that this logic was only intended for HTTP and HTTPS. I'd rather fall back on not changing the URL than changing it wrong (as I did with SSH)
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thockin, yyvess The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Followup to #868 from @yyvess
The test failed before #868 and passes now