Skip to content

Commit 0d50a8b

Browse files
AkihiroSudaAkihiroSuda
committed
git url: support query form
Fix issue 4905, but the syntax differs from the original proposal. The document will be added to https://github.com/docker/docs/blob/main/content/manuals/build/concepts/context.md#url-fragments Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
1 parent 27ebd19 commit 0d50a8b

5 files changed

Lines changed: 173 additions & 23 deletions

File tree

‎frontend/dockerfile/dfgitutil/git_ref.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,14 @@ func ParseGitRef(ref string) (*GitRef, error) {
6363
return nil, cerrdefs.ErrInvalidArgument
6464
} else if strings.HasPrefix(ref, "github.com/") {
6565
res.IndistinguishableFromLocal = true // Deprecated
66-
remote = gitutil.FromURL(&url.URL{
66+
remote, err = gitutil.FromURL(&url.URL{
6767
Scheme: "https",
6868
Host: "github.com",
6969
Path: strings.TrimPrefix(ref, "github.com/"),
7070
})
71+
if err != nil {
72+
return nil, err
73+
}
7174
} else {
7275
remote, err = gitutil.ParseURL(ref)
7376
if errors.Is(err, gitutil.ErrUnknownProtocol) {

‎frontend/dockerfile/dfgitutil/git_ref_test.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,15 @@ func TestParseGitRef(t *testing.T) {
138138
ref: ".git",
139139
expected: nil,
140140
},
141+
{
142+
ref: "https://github.com/docker/docker.git?ref=v1.0.0&subdir=/subdir",
143+
expected: &GitRef{
144+
Remote: "https://github.com/docker/docker.git",
145+
ShortName: "docker",
146+
Commit: "v1.0.0",
147+
SubDir: "/subdir",
148+
},
149+
},
141150
}
142151
for _, tt := range cases {
143152
t.Run(tt.ref, func(t *testing.T) {

‎util/gitutil/git_url.go‎

Lines changed: 75 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ type GitURL struct {
5656
}
5757

5858
// GitURLOpts is the buildkit-specific metadata extracted from the fragment
59-
// of a remote URL.
59+
// or the query of a remote URL.
6060
type GitURLOpts struct {
6161
// Ref is the git reference
6262
Ref string
@@ -66,12 +66,63 @@ type GitURLOpts struct {
6666

6767
// parseOpts splits a git URL fragment into its respective git
6868
// reference and subdirectory components.
69-
func parseOpts(fragment string) *GitURLOpts {
70-
if fragment == "" {
71-
return nil
69+
func parseOpts(fragment string, query url.Values) (*GitURLOpts, error) {
70+
if fragment == "" && len(query) == 0 {
71+
return nil, nil
7272
}
73-
ref, subdir, _ := strings.Cut(fragment, ":")
74-
return &GitURLOpts{Ref: ref, Subdir: subdir}
73+
opts := &GitURLOpts{}
74+
if fragment != "" {
75+
opts.Ref, opts.Subdir, _ = strings.Cut(fragment, ":")
76+
}
77+
var tag, branch string
78+
for k, v := range query {
79+
switch len(v) {
80+
case 0:
81+
return nil, errors.Errorf("query %q has no value", k)
82+
case 1:
83+
if v[0] == "" {
84+
return nil, errors.Errorf("query %q has no value", k)
85+
}
86+
// NOP
87+
default:
88+
return nil, errors.Errorf("query %q has multiple values", k)
89+
}
90+
switch k {
91+
case "ref":
92+
if opts.Ref != "" && opts.Ref != v[0] {
93+
return nil, errors.Errorf("ref conflicts: %q vs %q", opts.Ref, v[0])
94+
}
95+
opts.Ref = v[0]
96+
case "tag":
97+
tag = v[0]
98+
case "branch":
99+
branch = v[0]
100+
case "subdir":
101+
if opts.Subdir != "" && opts.Subdir != v[0] {
102+
return nil, errors.Errorf("subdir conflicts: %q vs %q", opts.Subdir, v[0])
103+
}
104+
opts.Subdir = v[0]
105+
default:
106+
return nil, errors.Errorf("unexpected query %q", k)
107+
}
108+
}
109+
if tag != "" {
110+
if opts.Ref != "" {
111+
return nil, errors.New("tag conflicts with ref")
112+
}
113+
opts.Ref = "refs/tags/" + tag
114+
}
115+
if branch != "" {
116+
if tag != "" {
117+
// TODO: consider allowing this, when the tag actually exists on the branch
118+
return nil, errors.New("branch conflicts with tag")
119+
}
120+
if opts.Ref != "" {
121+
return nil, errors.New("branch conflicts with ref")
122+
}
123+
opts.Ref = "refs/heads/" + branch
124+
}
125+
return opts, nil
75126
}
76127

77128
// ParseURL parses a BuildKit-style Git URL (that may contain additional
@@ -86,11 +137,11 @@ func ParseURL(remote string) (*GitURL, error) {
86137
if err != nil {
87138
return nil, err
88139
}
89-
return FromURL(url), nil
140+
return FromURL(url)
90141
}
91142

92143
if url, err := sshutil.ParseSCPStyleURL(remote); err == nil {
93-
return fromSCPStyleURL(url), nil
144+
return fromSCPStyleURL(url)
94145
}
95146

96147
return nil, ErrUnknownProtocol
@@ -105,28 +156,38 @@ func IsGitTransport(remote string) bool {
105156
return sshutil.IsImplicitSSHTransport(remote)
106157
}
107158

108-
func FromURL(url *url.URL) *GitURL {
159+
func FromURL(url *url.URL) (*GitURL, error) {
109160
withoutOpts := *url
110161
withoutOpts.Fragment = ""
162+
withoutOpts.RawQuery = ""
163+
opts, err := parseOpts(url.Fragment, url.Query())
164+
if err != nil {
165+
return nil, err
166+
}
111167
return &GitURL{
112168
Scheme: url.Scheme,
113169
User: url.User,
114170
Host: url.Host,
115171
Path: url.Path,
116-
Opts: parseOpts(url.Fragment),
172+
Opts: opts,
117173
Remote: withoutOpts.String(),
118-
}
174+
}, nil
119175
}
120176

121-
func fromSCPStyleURL(url *sshutil.SCPStyleURL) *GitURL {
177+
func fromSCPStyleURL(url *sshutil.SCPStyleURL) (*GitURL, error) {
122178
withoutOpts := *url
123179
withoutOpts.Fragment = ""
180+
withoutOpts.Query = nil
181+
opts, err := parseOpts(url.Fragment, url.Query)
182+
if err != nil {
183+
return nil, err
184+
}
124185
return &GitURL{
125186
Scheme: SSHProtocol,
126187
User: url.User,
127188
Host: url.Host,
128189
Path: url.Path,
129-
Opts: parseOpts(url.Fragment),
190+
Opts: opts,
130191
Remote: withoutOpts.String(),
131-
}
192+
}, nil
132193
}

‎util/gitutil/git_url_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,65 @@ func TestParseURL(t *testing.T) {
151151
Path: "/moby/buildkit",
152152
},
153153
},
154+
{
155+
url: "https://github.com/moby/buildkit?ref=v1.0.0&subdir=/subdir",
156+
result: GitURL{
157+
Scheme: HTTPSProtocol,
158+
Host: "github.com",
159+
Path: "/moby/buildkit",
160+
Opts: &GitURLOpts{Ref: "v1.0.0", Subdir: "/subdir"},
161+
},
162+
},
163+
{
164+
url: "https://github.com/moby/buildkit?subdir=/subdir#v1.0.0",
165+
result: GitURL{
166+
Scheme: HTTPSProtocol,
167+
Host: "github.com",
168+
Path: "/moby/buildkit",
169+
Opts: &GitURLOpts{Ref: "v1.0.0", Subdir: "/subdir"},
170+
},
171+
},
172+
{
173+
url: "https://github.com/moby/buildkit?tag=v1.0.0",
174+
result: GitURL{
175+
Scheme: HTTPSProtocol,
176+
Host: "github.com",
177+
Path: "/moby/buildkit",
178+
Opts: &GitURLOpts{Ref: "refs/tags/v1.0.0"},
179+
},
180+
},
181+
{
182+
url: "https://github.com/moby/buildkit?branch=v1.0",
183+
result: GitURL{
184+
Scheme: HTTPSProtocol,
185+
Host: "github.com",
186+
Path: "/moby/buildkit",
187+
Opts: &GitURLOpts{Ref: "refs/heads/v1.0"},
188+
},
189+
},
190+
{
191+
url: "https://github.com/moby/buildkit?ref=v1.0.0#v1.2.3",
192+
err: true,
193+
},
194+
{
195+
url: "https://github.com/moby/buildkit?ref=v1.0.0&tag=v1.2.3",
196+
err: true,
197+
},
198+
{
199+
// TODO: consider allowing this, when the tag actually exists on the branch
200+
url: "https://github.com/moby/buildkit?tag=v1.0.0&branch=v1.0",
201+
err: true,
202+
},
203+
{
204+
url: "git@github.com:moby/buildkit.git?subdir=/subdir#v1.0.0",
205+
result: GitURL{
206+
Scheme: SSHProtocol,
207+
Host: "github.com",
208+
Path: "moby/buildkit.git",
209+
User: url.User("git"),
210+
Opts: &GitURLOpts{Ref: "v1.0.0", Subdir: "/subdir"},
211+
},
212+
},
154213
}
155214
for _, test := range tests {
156215
t.Run(test.url, func(t *testing.T) {

‎util/sshutil/scpurl.go‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
package sshutil
22

33
import (
4-
"errors"
54
"fmt"
65
"net/url"
76
"regexp"
7+
8+
"github.com/pkg/errors"
89
)
910

10-
var gitSSHRegex = regexp.MustCompile("^([a-zA-Z0-9-_]+)@([a-zA-Z0-9-.]+):(.*?)(?:#(.*))?$")
11+
var gitSSHRegex = regexp.MustCompile(`^([a-zA-Z0-9-_]+)@([a-zA-Z0-9-.]+):(.*?)(?:\?(.*?))?(?:#(.*))?$`)
1112

1213
func IsImplicitSSHTransport(s string) bool {
1314
return gitSSHRegex.MatchString(s)
@@ -18,6 +19,7 @@ type SCPStyleURL struct {
1819
Host string
1920

2021
Path string
22+
Query url.Values
2123
Fragment string
2224
}
2325

@@ -26,18 +28,34 @@ func ParseSCPStyleURL(raw string) (*SCPStyleURL, error) {
2628
if matches == nil {
2729
return nil, errors.New("invalid scp-style url")
2830
}
31+
32+
rawQuery := matches[4]
33+
vals := url.Values{}
34+
if rawQuery != "" {
35+
var err error
36+
vals, err = url.ParseQuery(rawQuery)
37+
if err != nil {
38+
return nil, errors.Wrap(err, "invalid query in scp-style url")
39+
}
40+
}
41+
2942
return &SCPStyleURL{
3043
User: url.User(matches[1]),
3144
Host: matches[2],
3245
Path: matches[3],
33-
Fragment: matches[4],
46+
Query: vals,
47+
Fragment: matches[5],
3448
}, nil
3549
}
3650

37-
func (url *SCPStyleURL) String() string {
38-
base := fmt.Sprintf("%s@%s:%s", url.User.String(), url.Host, url.Path)
39-
if url.Fragment == "" {
40-
return base
51+
func (u *SCPStyleURL) String() string {
52+
s := fmt.Sprintf("%s@%s:%s", u.User.String(), u.Host, u.Path)
53+
54+
if len(u.Query) > 0 {
55+
s += "?" + u.Query.Encode()
56+
}
57+
if u.Fragment != "" {
58+
s += "#" + u.Fragment
4159
}
42-
return base + "#" + url.Fragment
60+
return s
4361
}

0 commit comments

Comments
 (0)