Skip to content

Commit 50e75e3

Browse files
jedevcaaronlehmann
jedevc
and
aaronlehmann
committed
git: update parsing to clarify between scp-style urls
This should also resolve the ports parsing issue. Co-authored-by: Aaron Lehmann <[email protected]> Signed-off-by: Justin Chadwell <[email protected]>
1 parent e84cc9e commit 50e75e3

File tree

9 files changed

+276
-173
lines changed

9 files changed

+276
-173
lines changed

‎client/llb/source.go

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
_ "crypto/sha256" // for opencontainers/go-digest
66
"encoding/json"
77
"os"
8+
"path"
89
"strconv"
910
"strings"
1011

@@ -237,29 +238,30 @@ type ImageInfo struct {
237238
//
238239
// By default the git repository is cloned with `--depth=1` to reduce the amount of data downloaded.
239240
// Additionally the ".git" directory is removed after the clone, you can keep ith with the [KeepGitDir] [GitOption].
240-
func Git(remote, ref string, opts ...GitOption) State {
241-
url := strings.Split(remote, "#")[0]
242-
243-
var protocolType int
244-
remote, protocolType = gitutil.ParseProtocol(remote)
245-
246-
var sshHost string
247-
if protocolType == gitutil.SSHProtocol {
248-
parts := strings.SplitN(remote, ":", 2)
249-
if len(parts) == 2 {
250-
sshHost = parts[0]
251-
// keep remote consistent with http(s) version
252-
remote = parts[0] + "/" + parts[1]
253-
}
254-
}
255-
if protocolType == gitutil.UnknownProtocol {
241+
func Git(url, ref string, opts ...GitOption) State {
242+
remote, err := gitutil.ParseURL(url)
243+
if errors.Is(err, gitutil.ErrUnknownProtocol) {
256244
url = "https://" + url
245+
remote, err = gitutil.ParseURL(url)
246+
}
247+
if remote != nil {
248+
remote.Fragment = ""
249+
url = remote.String()
257250
}
258251

259-
id := remote
260-
261-
if ref != "" {
262-
id += "#" + ref
252+
var id string
253+
if err != nil {
254+
// If we can't parse the URL, just use the full URL as the ID. The git
255+
// operation will fail later on.
256+
id = url
257+
} else {
258+
// We construct the ID manually here, so that we can create the same ID
259+
// for different protocols (e.g. https and ssh) that have the same
260+
// host/path/fragment combination.
261+
id = remote.Host + path.Join("/", remote.Path)
262+
if ref != "" {
263+
id += "#" + ref
264+
}
263265
}
264266

265267
gi := &GitInfo{
@@ -290,11 +292,11 @@ func Git(remote, ref string, opts ...GitOption) State {
290292
addCap(&gi.Constraints, pb.CapSourceGitHTTPAuth)
291293
}
292294
}
293-
if protocolType == gitutil.SSHProtocol {
295+
if remote != nil && remote.Scheme == gitutil.SSHProtocol {
294296
if gi.KnownSSHHosts != "" {
295297
attrs[pb.AttrKnownSSHHosts] = gi.KnownSSHHosts
296-
} else if sshHost != "" {
297-
keyscan, err := sshutil.SSHKeyScan(sshHost)
298+
} else {
299+
keyscan, err := sshutil.SSHKeyScan(remote.Host)
298300
if err == nil {
299301
// best effort
300302
attrs[pb.AttrKnownSSHHosts] = keyscan

‎source/git/identifier.go

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package git
22

33
import (
4-
"net/url"
54
"path"
65
"strings"
76

87
"github.com/moby/buildkit/solver/llbsolver/provenance"
98
"github.com/moby/buildkit/source"
109
srctypes "github.com/moby/buildkit/source/types"
10+
"github.com/moby/buildkit/util/gitutil"
1111
"github.com/moby/buildkit/util/sshutil"
1212
)
1313

@@ -23,32 +23,19 @@ type GitIdentifier struct {
2323
}
2424

2525
func NewGitIdentifier(remoteURL string) (*GitIdentifier, error) {
26-
repo := GitIdentifier{}
27-
2826
if !isGitTransport(remoteURL) {
2927
remoteURL = "https://" + remoteURL
3028
}
29+
u, err := gitutil.ParseURL(remoteURL)
30+
if err != nil {
31+
return nil, err
32+
}
3133

32-
var fragment string
33-
if sshutil.IsImplicitSSHTransport(remoteURL) {
34-
// implicit ssh urls such as "git@.." are not actually a URL, so cannot be parsed as URL
35-
parts := strings.SplitN(remoteURL, "#", 2)
36-
37-
repo.Remote = parts[0]
38-
if len(parts) == 2 {
39-
fragment = parts[1]
40-
}
41-
repo.Ref, repo.Subdir = getRefAndSubdir(fragment)
42-
} else {
43-
u, err := url.Parse(remoteURL)
44-
if err != nil {
45-
return nil, err
46-
}
34+
repo := GitIdentifier{}
35+
repo.Ref, repo.Subdir = gitutil.SplitGitFragment(u.Fragment)
36+
u.Fragment = ""
37+
repo.Remote = u.String()
4738

48-
repo.Ref, repo.Subdir = getRefAndSubdir(u.Fragment)
49-
u.Fragment = ""
50-
repo.Remote = u.String()
51-
}
5239
if sd := path.Clean(repo.Subdir); sd == "/" || sd == "." {
5340
repo.Subdir = ""
5441
}
@@ -96,15 +83,3 @@ func (id *GitIdentifier) Capture(c *provenance.Capture, pin string) error {
9683
func isGitTransport(str string) bool {
9784
return strings.HasPrefix(str, "http://") || strings.HasPrefix(str, "https://") || strings.HasPrefix(str, "git://") || strings.HasPrefix(str, "ssh://") || sshutil.IsImplicitSSHTransport(str)
9885
}
99-
100-
func getRefAndSubdir(fragment string) (ref string, subdir string) {
101-
refAndDir := strings.SplitN(fragment, ":", 2)
102-
ref = ""
103-
if len(refAndDir[0]) != 0 {
104-
ref = refAndDir[0]
105-
}
106-
if len(refAndDir) > 1 && len(refAndDir[1]) != 0 {
107-
subdir = refAndDir[1]
108-
}
109-
return
110-
}

‎source/git/identifier_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func TestNewGitIdentifier(t *testing.T) {
2727
{
2828
url: "[email protected]:moby/buildkit.git",
2929
expected: GitIdentifier{
30-
Remote: "[email protected]:moby/buildkit.git",
30+
Remote: "ssh://[email protected]/moby/buildkit.git",
3131
},
3232
},
3333
{
@@ -75,13 +75,13 @@ func TestNewGitIdentifier(t *testing.T) {
7575
{
7676
url: "[email protected]:user/repo.git",
7777
expected: GitIdentifier{
78-
Remote: "[email protected]:user/repo.git",
78+
Remote: "ssh://[email protected]/user/repo.git",
7979
},
8080
},
8181
{
8282
url: "[email protected]:user/repo.git#mybranch:mydir/mysubdir/",
8383
expected: GitIdentifier{
84-
Remote: "[email protected]:user/repo.git",
84+
Remote: "ssh://[email protected]/user/repo.git",
8585
Ref: "mybranch",
8686
Subdir: "mydir/mysubdir/",
8787
},

‎util/gitutil/git_protocol.go

Lines changed: 0 additions & 46 deletions
This file was deleted.

‎util/gitutil/git_protocol_test.go

Lines changed: 0 additions & 53 deletions
This file was deleted.

‎util/gitutil/git_ref.go

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package gitutil
22

33
import (
4+
"net/url"
45
"regexp"
56
"strings"
67

78
"github.com/containerd/containerd/errdefs"
9+
"github.com/pkg/errors"
810
)
911

1012
// GitRef represents a git ref.
@@ -51,19 +53,33 @@ type GitRef struct {
5153
func ParseGitRef(ref string) (*GitRef, error) {
5254
res := &GitRef{}
5355

56+
var (
57+
remote *url.URL
58+
err error
59+
)
60+
5461
if strings.HasPrefix(ref, "github.com/") {
5562
res.IndistinguishableFromLocal = true // Deprecated
63+
remote = &url.URL{
64+
Scheme: "https",
65+
Host: "github.com",
66+
Path: strings.TrimPrefix(ref, "github.com/"),
67+
}
5668
} else {
57-
_, proto := ParseProtocol(ref)
58-
switch proto {
59-
case UnknownProtocol:
60-
return nil, errdefs.ErrInvalidArgument
69+
remote, err = ParseURL(ref)
70+
if errors.Is(err, ErrUnknownProtocol) {
71+
remote, err = ParseURL("https://" + ref)
6172
}
62-
switch proto {
73+
if err != nil {
74+
return nil, err
75+
}
76+
77+
switch remote.Scheme {
6378
case HTTPProtocol, GitProtocol:
6479
res.UnencryptedTCP = true // Discouraged, but not deprecated
6580
}
66-
switch proto {
81+
82+
switch remote.Scheme {
6783
// An HTTP(S) URL is considered to be a valid git ref only when it has the ".git[...]" suffix.
6884
case HTTPProtocol, HTTPSProtocol:
6985
var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
@@ -73,13 +89,16 @@ func ParseGitRef(ref string) (*GitRef, error) {
7389
}
7490
}
7591

76-
var fragment string
77-
res.Remote, fragment, _ = strings.Cut(ref, "#")
78-
if len(res.Remote) == 0 {
79-
return res, errdefs.ErrInvalidArgument
92+
res.Commit, res.SubDir = SplitGitFragment(remote.Fragment)
93+
remote.Fragment = ""
94+
95+
res.Remote = remote.String()
96+
if res.IndistinguishableFromLocal {
97+
_, res.Remote, _ = strings.Cut(res.Remote, "://")
8098
}
81-
res.Commit, res.SubDir, _ = strings.Cut(fragment, ":")
99+
82100
repoSplitBySlash := strings.Split(res.Remote, "/")
83101
res.ShortName = strings.TrimSuffix(repoSplitBySlash[len(repoSplitBySlash)-1], ".git")
102+
84103
return res, nil
85104
}

‎util/gitutil/git_ref_test.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@ func TestParseGitRef(t *testing.T) {
6666
IndistinguishableFromLocal: true,
6767
},
6868
},
69+
{
70+
ref: "custom.xyz/moby/buildkit.git",
71+
expected: &GitRef{
72+
Remote: "https://custom.xyz/moby/buildkit.git",
73+
ShortName: "buildkit",
74+
},
75+
},
6976
{
7077
ref: "https://github.com/moby/buildkit",
7178
expected: nil,
@@ -77,24 +84,31 @@ func TestParseGitRef(t *testing.T) {
7784
ShortName: "buildkit",
7885
},
7986
},
87+
{
88+
ref: "https://foo:[email protected]/moby/buildkit.git",
89+
expected: &GitRef{
90+
Remote: "https://foo:[email protected]/moby/buildkit.git",
91+
ShortName: "buildkit",
92+
},
93+
},
8094
{
8195
ref: "[email protected]:moby/buildkit",
8296
expected: &GitRef{
83-
Remote: "[email protected]:moby/buildkit",
97+
Remote: "ssh://[email protected]/moby/buildkit",
8498
ShortName: "buildkit",
8599
},
86100
},
87101
{
88102
ref: "[email protected]:moby/buildkit.git",
89103
expected: &GitRef{
90-
Remote: "[email protected]:moby/buildkit.git",
104+
Remote: "ssh://[email protected]/moby/buildkit.git",
91105
ShortName: "buildkit",
92106
},
93107
},
94108
{
95109
ref: "[email protected]:atlassianlabs/atlassian-docker.git",
96110
expected: &GitRef{
97-
Remote: "[email protected]:atlassianlabs/atlassian-docker.git",
111+
Remote: "ssh://[email protected]/atlassianlabs/atlassian-docker.git",
98112
ShortName: "atlassian-docker",
99113
},
100114
},

0 commit comments

Comments
 (0)