Skip to content

Commit 99a9103

Browse files
AkihiroSudaAkihiroSuda
authored
Merge pull request #6172 from tonistiigi/git-querystring
Support querystring form Git URLs
2 parents 79c5e53 + 3afd92a commit 99a9103

11 files changed

Lines changed: 673 additions & 69 deletions

File tree

‎client/llb/git_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package llb
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/moby/buildkit/solver/pb"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestGit(t *testing.T) {
12+
t.Parallel()
13+
14+
type tcase struct {
15+
name string
16+
st State
17+
identifier string
18+
attrs map[string]string
19+
}
20+
21+
tcases := []tcase{
22+
{
23+
name: "refarg",
24+
st: Git("github.com/foo/bar.git", "ref"),
25+
identifier: "git://github.com/foo/bar.git#ref",
26+
attrs: map[string]string{
27+
"git.authheadersecret": "GIT_AUTH_HEADER",
28+
"git.authtokensecret": "GIT_AUTH_TOKEN",
29+
"git.fullurl": "https://github.com/foo/bar.git",
30+
},
31+
},
32+
{
33+
name: "refarg with subdir",
34+
st: Git("github.com/foo/bar.git", "ref:subdir"),
35+
identifier: "git://github.com/foo/bar.git#ref:subdir",
36+
attrs: map[string]string{
37+
"git.authheadersecret": "GIT_AUTH_HEADER",
38+
"git.authtokensecret": "GIT_AUTH_TOKEN",
39+
"git.fullurl": "https://github.com/foo/bar.git",
40+
},
41+
},
42+
{
43+
name: "refarg with subdir func",
44+
st: Git("github.com/foo/bar.git", "ref", GitSubDir("subdir")),
45+
identifier: "git://github.com/foo/bar.git#ref:subdir",
46+
attrs: map[string]string{
47+
"git.authheadersecret": "GIT_AUTH_HEADER",
48+
"git.authtokensecret": "GIT_AUTH_TOKEN",
49+
"git.fullurl": "https://github.com/foo/bar.git",
50+
},
51+
},
52+
{
53+
name: "refarg with override",
54+
st: Git("github.com/foo/bar.git", "ref:dir", GitRef("v1.0")),
55+
identifier: "git://github.com/foo/bar.git#v1.0:dir",
56+
attrs: map[string]string{
57+
"git.authheadersecret": "GIT_AUTH_HEADER",
58+
"git.authtokensecret": "GIT_AUTH_TOKEN",
59+
"git.fullurl": "https://github.com/foo/bar.git",
60+
},
61+
},
62+
{
63+
name: "funcs only",
64+
st: Git("github.com/foo/bar.git", "", GitRef("v1.0"), GitSubDir("dir")),
65+
identifier: "git://github.com/foo/bar.git#v1.0:dir",
66+
attrs: map[string]string{
67+
"git.authheadersecret": "GIT_AUTH_HEADER",
68+
"git.authtokensecret": "GIT_AUTH_TOKEN",
69+
"git.fullurl": "https://github.com/foo/bar.git",
70+
},
71+
},
72+
}
73+
74+
for _, tc := range tcases {
75+
t.Run(tc.name, func(t *testing.T) {
76+
st := tc.st
77+
def, err := st.Marshal(context.TODO())
78+
79+
require.NoError(t, err)
80+
81+
m, arr := parseDef(t, def.Def)
82+
require.Equal(t, 2, len(arr))
83+
84+
dgst, idx := last(t, arr)
85+
require.Equal(t, 0, idx)
86+
require.Equal(t, m[dgst], arr[0])
87+
88+
g := arr[0].Op.(*pb.Op_Source).Source
89+
90+
require.Equal(t, tc.identifier, g.Identifier)
91+
require.Equal(t, tc.attrs, g.Attrs)
92+
})
93+
}
94+
}

‎client/llb/source.go‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,13 @@ const (
247247
// Formats that utilize SSH may need to supply credentials as a [GitOption].
248248
// You may need to check the source code for a full list of supported formats.
249249
//
250+
// Fragment can be used to pass ref:subdir format that can set in (old-style)
251+
// Docker Git URL format after # . This is provided for backwards compatibility.
252+
// It is recommended to leave it empty and call GitRef(), GitSubdir() options instead.
253+
//
250254
// By default the git repository is cloned with `--depth=1` to reduce the amount of data downloaded.
251255
// Additionally the ".git" directory is removed after the clone, you can keep ith with the [KeepGitDir] [GitOption].
252-
func Git(url, ref string, opts ...GitOption) State {
256+
func Git(url, fragment string, opts ...GitOption) State {
253257
remote, err := gitutil.ParseURL(url)
254258
if errors.Is(err, gitutil.ErrUnknownProtocol) {
255259
url = "https://" + url
@@ -259,6 +263,20 @@ func Git(url, ref string, opts ...GitOption) State {
259263
url = remote.Remote
260264
}
261265

266+
gi := &GitInfo{
267+
AuthHeaderSecret: GitAuthHeaderKey,
268+
AuthTokenSecret: GitAuthTokenKey,
269+
}
270+
ref, subdir, ok := strings.Cut(fragment, ":")
271+
if ref != "" {
272+
GitRef(ref).SetGitOption(gi)
273+
}
274+
if ok && subdir != "" {
275+
GitSubDir(subdir).SetGitOption(gi)
276+
}
277+
for _, o := range opts {
278+
o.SetGitOption(gi)
279+
}
262280
var id string
263281
if err != nil {
264282
// If we can't parse the URL, just use the full URL as the ID. The git
@@ -269,18 +287,13 @@ func Git(url, ref string, opts ...GitOption) State {
269287
// for different protocols (e.g. https and ssh) that have the same
270288
// host/path/fragment combination.
271289
id = remote.Host + path.Join("/", remote.Path)
272-
if ref != "" {
273-
id += "#" + ref
290+
if gi.Ref != "" || gi.SubDir != "" {
291+
id += "#" + gi.Ref
292+
if gi.SubDir != "" {
293+
id += ":" + gi.SubDir
294+
}
274295
}
275296
}
276-
277-
gi := &GitInfo{
278-
AuthHeaderSecret: GitAuthHeaderKey,
279-
AuthTokenSecret: GitAuthTokenKey,
280-
}
281-
for _, o := range opts {
282-
o.SetGitOption(gi)
283-
}
284297
attrs := map[string]string{}
285298
if gi.KeepGitDir {
286299
attrs[pb.AttrKeepGitDir] = "true"
@@ -352,6 +365,20 @@ type GitInfo struct {
352365
KnownSSHHosts string
353366
MountSSHSock string
354367
Checksum string
368+
Ref string
369+
SubDir string
370+
}
371+
372+
func GitRef(v string) GitOption {
373+
return gitOptionFunc(func(gi *GitInfo) {
374+
gi.Ref = v
375+
})
376+
}
377+
378+
func GitSubDir(v string) GitOption {
379+
return gitOptionFunc(func(gi *GitInfo) {
380+
gi.SubDir = v
381+
})
355382
}
356383

357384
func KeepGitDir() GitOption {

‎frontend/dockerfile/dfgitutil/git_ref.go‎

Lines changed: 94 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,12 @@ type GitRef struct {
2323
// e.g., "bar" for "https://github.com/foo/bar.git"
2424
ShortName string
2525

26-
// Commit is a commit hash, a tag, or branch name.
27-
// Commit is optional.
28-
Commit string
26+
// Ref is a commit hash, a tag, or branch name.
27+
// Ref is optional.
28+
Ref string
29+
30+
// Checksum is a commit hash.
31+
Checksum string
2932

3033
// SubDir is a directory path inside the repo.
3134
// SubDir is optional.
@@ -48,10 +51,8 @@ type GitRef struct {
4851
UnencryptedTCP bool
4952
}
5053

51-
// var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
52-
5354
// ParseGitRef parses a git ref.
54-
func ParseGitRef(ref string) (*GitRef, error) {
55+
func ParseGitRef(ref string) (*GitRef, bool, error) {
5556
res := &GitRef{}
5657

5758
var (
@@ -60,21 +61,25 @@ func ParseGitRef(ref string) (*GitRef, error) {
6061
)
6162

6263
if strings.HasPrefix(ref, "./") || strings.HasPrefix(ref, "../") {
63-
return nil, cerrdefs.ErrInvalidArgument
64+
return nil, false, errors.WithStack(cerrdefs.ErrInvalidArgument)
6465
} else if strings.HasPrefix(ref, "github.com/") {
6566
res.IndistinguishableFromLocal = true // Deprecated
66-
remote = gitutil.FromURL(&url.URL{
67-
Scheme: "https",
68-
Host: "github.com",
69-
Path: strings.TrimPrefix(ref, "github.com/"),
70-
})
67+
u, err := url.Parse(ref)
68+
if err != nil {
69+
return nil, false, err
70+
}
71+
u.Scheme = "https"
72+
remote, err = gitutil.FromURL(u)
73+
if err != nil {
74+
return nil, false, err
75+
}
7176
} else {
7277
remote, err = gitutil.ParseURL(ref)
7378
if errors.Is(err, gitutil.ErrUnknownProtocol) {
74-
return nil, err
79+
return nil, false, err
7580
}
7681
if err != nil {
77-
return nil, err
82+
return nil, false, err
7883
}
7984

8085
switch remote.Scheme {
@@ -86,7 +91,7 @@ func ParseGitRef(ref string) (*GitRef, error) {
8691
// An HTTP(S) URL is considered to be a valid git ref only when it has the ".git[...]" suffix.
8792
case gitutil.HTTPProtocol, gitutil.HTTPSProtocol:
8893
if !strings.HasSuffix(remote.Path, ".git") {
89-
return nil, cerrdefs.ErrInvalidArgument
94+
return nil, false, errors.WithStack(cerrdefs.ErrInvalidArgument)
9095
}
9196
}
9297
}
@@ -96,11 +101,83 @@ func ParseGitRef(ref string) (*GitRef, error) {
96101
_, res.Remote, _ = strings.Cut(res.Remote, "://")
97102
}
98103
if remote.Opts != nil {
99-
res.Commit, res.SubDir = remote.Opts.Ref, remote.Opts.Subdir
104+
res.Ref, res.SubDir = remote.Opts.Ref, remote.Opts.Subdir
100105
}
101106

102107
repoSplitBySlash := strings.Split(res.Remote, "/")
103108
res.ShortName = strings.TrimSuffix(repoSplitBySlash[len(repoSplitBySlash)-1], ".git")
104109

105-
return res, nil
110+
if err := res.loadQuery(remote.Query); err != nil {
111+
return nil, true, err
112+
}
113+
114+
return res, true, nil
115+
}
116+
117+
func (gf *GitRef) loadQuery(query url.Values) error {
118+
if len(query) == 0 {
119+
return nil
120+
}
121+
var tag, branch string
122+
for k, v := range query {
123+
switch len(v) {
124+
case 0:
125+
return errors.Errorf("query %q has no value", k)
126+
case 1:
127+
if v[0] == "" {
128+
return errors.Errorf("query %q has no value", k)
129+
}
130+
// NOP
131+
default:
132+
return errors.Errorf("query %q has multiple values", k)
133+
}
134+
switch k {
135+
case "ref":
136+
if gf.Ref != "" && gf.Ref != v[0] {
137+
return errors.Errorf("ref conflicts: %q vs %q", gf.Ref, v[0])
138+
}
139+
gf.Ref = v[0]
140+
case "tag":
141+
tag = v[0]
142+
case "branch":
143+
branch = v[0]
144+
case "subdir":
145+
if gf.SubDir != "" && gf.SubDir != v[0] {
146+
return errors.Errorf("subdir conflicts: %q vs %q", gf.SubDir, v[0])
147+
}
148+
gf.SubDir = v[0]
149+
case "checksum", "commit":
150+
gf.Checksum = v[0]
151+
default:
152+
return errors.Errorf("unexpected query %q", k)
153+
}
154+
}
155+
if tag != "" {
156+
const tagPrefix = "refs/tags/"
157+
if !strings.HasPrefix(tag, tagPrefix) {
158+
tag = tagPrefix + tag
159+
}
160+
if gf.Ref != "" && gf.Ref != tag {
161+
return errors.Errorf("ref conflicts: %q vs %q", gf.Ref, tag)
162+
}
163+
gf.Ref = tag
164+
}
165+
if branch != "" {
166+
if tag != "" {
167+
// TODO: consider allowing this, when the tag actually exists on the branch
168+
return errors.New("branch conflicts with tag")
169+
}
170+
const branchPrefix = "refs/heads/"
171+
if !strings.HasPrefix(branch, branchPrefix) {
172+
branch = branchPrefix + branch
173+
}
174+
if gf.Ref != "" && gf.Ref != branch {
175+
return errors.Errorf("ref conflicts: %q vs %q", gf.Ref, branch)
176+
}
177+
gf.Ref = branch
178+
}
179+
if gf.Checksum != "" && gf.Ref == "" {
180+
gf.Ref = gf.Checksum
181+
}
182+
return nil
106183
}

0 commit comments

Comments
 (0)