Skip to content

Commit 8159942

Browse files
tonistiigitonistiigi
committed
llb: update Git to allow normalized property passing
Signed-off-by: Tonis Tiigi <[email protected]>
1 parent c8ce372 commit 8159942

File tree

7 files changed

+177
-44
lines changed

7 files changed

+177
-44
lines changed

‎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: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ const (
249249
//
250250
// By default the git repository is cloned with `--depth=1` to reduce the amount of data downloaded.
251251
// 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 {
252+
func Git(url, fragment string, opts ...GitOption) State {
253253
remote, err := gitutil.ParseURL(url)
254254
if errors.Is(err, gitutil.ErrUnknownProtocol) {
255255
url = "https://" + url
@@ -259,6 +259,20 @@ func Git(url, ref string, opts ...GitOption) State {
259259
url = remote.Remote
260260
}
261261

262+
gi := &GitInfo{
263+
AuthHeaderSecret: GitAuthHeaderKey,
264+
AuthTokenSecret: GitAuthTokenKey,
265+
}
266+
ref, subdir, ok := strings.Cut(fragment, ":")
267+
if ref != "" {
268+
GitRef(ref).SetGitOption(gi)
269+
}
270+
if ok && subdir != "" {
271+
GitSubDir(subdir).SetGitOption(gi)
272+
}
273+
for _, o := range opts {
274+
o.SetGitOption(gi)
275+
}
262276
var id string
263277
if err != nil {
264278
// If we can't parse the URL, just use the full URL as the ID. The git
@@ -269,18 +283,13 @@ func Git(url, ref string, opts ...GitOption) State {
269283
// for different protocols (e.g. https and ssh) that have the same
270284
// host/path/fragment combination.
271285
id = remote.Host + path.Join("/", remote.Path)
272-
if ref != "" {
273-
id += "#" + ref
286+
if gi.Ref != "" || gi.SubDir != "" {
287+
id += "#" + gi.Ref
288+
if gi.SubDir != "" {
289+
id += ":" + gi.SubDir
290+
}
274291
}
275292
}
276-
277-
gi := &GitInfo{
278-
AuthHeaderSecret: GitAuthHeaderKey,
279-
AuthTokenSecret: GitAuthTokenKey,
280-
}
281-
for _, o := range opts {
282-
o.SetGitOption(gi)
283-
}
284293
attrs := map[string]string{}
285294
if gi.KeepGitDir {
286295
attrs[pb.AttrKeepGitDir] = "true"
@@ -352,6 +361,20 @@ type GitInfo struct {
352361
KnownSSHHosts string
353362
MountSSHSock string
354363
Checksum string
364+
Ref string
365+
SubDir string
366+
}
367+
368+
func GitRef(v string) GitOption {
369+
return gitOptionFunc(func(gi *GitInfo) {
370+
gi.Ref = v
371+
})
372+
}
373+
374+
func GitSubDir(v string) GitOption {
375+
return gitOptionFunc(func(gi *GitInfo) {
376+
gi.SubDir = v
377+
})
355378
}
356379

357380
func KeepGitDir() GitOption {

‎frontend/dockerfile/dfgitutil/git_ref.go‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
package dfgitutil
33

44
import (
5-
"log"
65
"net/url"
76
"strings"
87

@@ -52,10 +51,8 @@ type GitRef struct {
5251
UnencryptedTCP bool
5352
}
5453

55-
// var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
56-
5754
// ParseGitRef parses a git ref.
58-
func ParseGitRef(ref string) (*GitRef, error) {
55+
func ParseGitRef(ref string) (*GitRef, bool, error) {
5956
res := &GitRef{}
6057

6158
var (
@@ -64,25 +61,25 @@ func ParseGitRef(ref string) (*GitRef, error) {
6461
)
6562

6663
if strings.HasPrefix(ref, "./") || strings.HasPrefix(ref, "../") {
67-
return nil, errors.WithStack(cerrdefs.ErrInvalidArgument)
64+
return nil, false, errors.WithStack(cerrdefs.ErrInvalidArgument)
6865
} else if strings.HasPrefix(ref, "github.com/") {
6966
res.IndistinguishableFromLocal = true // Deprecated
7067
u, err := url.Parse(ref)
7168
if err != nil {
72-
return nil, err
69+
return nil, false, err
7370
}
7471
u.Scheme = "https"
7572
remote, err = gitutil.FromURL(u)
7673
if err != nil {
77-
return nil, err
74+
return nil, false, err
7875
}
7976
} else {
8077
remote, err = gitutil.ParseURL(ref)
8178
if errors.Is(err, gitutil.ErrUnknownProtocol) {
82-
return nil, err
79+
return nil, false, err
8380
}
8481
if err != nil {
85-
return nil, err
82+
return nil, false, err
8683
}
8784

8885
switch remote.Scheme {
@@ -94,7 +91,7 @@ func ParseGitRef(ref string) (*GitRef, error) {
9491
// An HTTP(S) URL is considered to be a valid git ref only when it has the ".git[...]" suffix.
9592
case gitutil.HTTPProtocol, gitutil.HTTPSProtocol:
9693
if !strings.HasSuffix(remote.Path, ".git") {
97-
return nil, errors.WithStack(cerrdefs.ErrInvalidArgument)
94+
return nil, false, errors.WithStack(cerrdefs.ErrInvalidArgument)
9895
}
9996
}
10097
}
@@ -111,11 +108,10 @@ func ParseGitRef(ref string) (*GitRef, error) {
111108
res.ShortName = strings.TrimSuffix(repoSplitBySlash[len(repoSplitBySlash)-1], ".git")
112109

113110
if err := res.loadQuery(remote.Query); err != nil {
114-
log.Printf("query")
115-
return nil, err
111+
return nil, true, err
116112
}
117113

118-
return res, nil
114+
return res, true, nil
119115
}
120116

121117
func (gf *GitRef) loadQuery(query url.Values) error {

‎frontend/dockerfile/dfgitutil/git_ref_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ func TestParseGitRef(t *testing.T) {
221221
}
222222
for i, tt := range cases {
223223
t.Run(fmt.Sprintf("case%d", i+1), func(t *testing.T) {
224-
got, err := ParseGitRef(tt.ref)
224+
got, _, err := ParseGitRef(tt.ref)
225225
if tt.expected == nil {
226226
require.Nil(t, got)
227227
require.Error(t, err)

‎frontend/dockerfile/dockerfile2llb/convert.go‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,24 +1507,30 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
15071507

15081508
for _, src := range cfg.params.SourcePaths {
15091509
commitMessage.WriteString(" " + src)
1510-
gitRef, gitRefErr := dfgitutil.ParseGitRef(src)
1510+
gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src)
1511+
if gitRefErr != nil && isGit {
1512+
return gitRefErr
1513+
}
15111514
if gitRefErr == nil && !gitRef.IndistinguishableFromLocal {
15121515
if !cfg.isAddCommand {
15131516
return errors.New("source can't be a git ref for COPY")
15141517
}
15151518
// TODO: print a warning (not an error) if gitRef.UnencryptedTCP is true
1516-
commit := gitRef.Ref
1517-
if gitRef.SubDir != "" {
1518-
commit += ":" + gitRef.SubDir
1519+
gitOptions := []llb.GitOption{
1520+
llb.WithCustomName(pgName),
1521+
llb.GitRef(gitRef.Ref),
15191522
}
1520-
gitOptions := []llb.GitOption{llb.WithCustomName(pgName)}
15211523
if cfg.keepGitDir {
15221524
gitOptions = append(gitOptions, llb.KeepGitDir())
15231525
}
15241526
if cfg.checksum != "" {
15251527
gitOptions = append(gitOptions, llb.GitChecksum(cfg.checksum))
15261528
}
1527-
st := llb.Git(gitRef.Remote, commit, gitOptions...)
1529+
if gitRef.SubDir != "" {
1530+
gitOptions = append(gitOptions, llb.GitSubDir(gitRef.SubDir))
1531+
}
1532+
1533+
st := llb.Git(gitRef.Remote, "", gitOptions...)
15281534
opts := append([]llb.CopyOption{&llb.CopyInfo{
15291535
Mode: chopt,
15301536
CreateDestPath: true,
@@ -2268,7 +2274,7 @@ func isHTTPSource(src string) bool {
22682274

22692275
func isGitSource(src string) bool {
22702276
// https://github.com/ORG/REPO.git is a git source, not an http source
2271-
if gitRef, gitErr := dfgitutil.ParseGitRef(src); gitRef != nil && gitErr == nil {
2277+
if gitRef, isGit, _ := dfgitutil.ParseGitRef(src); gitRef != nil && isGit {
22722278
return true
22732279
}
22742280
return false

‎frontend/dockerui/context.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ func (bc *Client) initContext(ctx context.Context) (*buildContext, error) {
7373
if v, err := strconv.ParseBool(opts[keyContextKeepGitDirArg]); err == nil {
7474
keepGit = v
7575
}
76-
if st, ok := DetectGitContext(opts[localNameContext], keepGit); ok {
76+
if st, ok, err := DetectGitContext(opts[localNameContext], keepGit); ok {
77+
if err != nil {
78+
return nil, err
79+
}
7780
bctx.context = st
7881
bctx.dockerfile = st
7982
} else if st, filename, ok := DetectHTTPContext(opts[localNameContext]); ok {
@@ -140,22 +143,27 @@ func (bc *Client) initContext(ctx context.Context) (*buildContext, error) {
140143
return bctx, nil
141144
}
142145

143-
func DetectGitContext(ref string, keepGit bool) (*llb.State, bool) {
144-
g, err := dfgitutil.ParseGitRef(ref)
146+
func DetectGitContext(ref string, keepGit bool) (*llb.State, bool, error) {
147+
g, isGit, err := dfgitutil.ParseGitRef(ref)
145148
if err != nil {
146-
return nil, false
149+
return nil, isGit, err
147150
}
148-
commit := g.Ref
149-
if g.SubDir != "" {
150-
commit += ":" + g.SubDir
151+
gitOpts := []llb.GitOption{
152+
llb.GitRef(g.Ref),
153+
WithInternalName("load git source " + ref),
151154
}
152-
gitOpts := []llb.GitOption{WithInternalName("load git source " + ref)}
153155
if keepGit {
154156
gitOpts = append(gitOpts, llb.KeepGitDir())
155157
}
158+
if g.SubDir != "" {
159+
gitOpts = append(gitOpts, llb.GitSubDir(g.SubDir))
160+
}
161+
if g.Checksum != "" {
162+
gitOpts = append(gitOpts, llb.GitChecksum(g.Checksum))
163+
}
156164

157-
st := llb.Git(g.Remote, commit, gitOpts...)
158-
return &st, true
165+
st := llb.Git(g.Remote, "", gitOpts...)
166+
return &st, true, nil
159167
}
160168

161169
func DetectHTTPContext(ref string) (*llb.State, string, bool) {

‎frontend/dockerui/namedcontext.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,17 +138,23 @@ func (nc *NamedContext) load(ctx context.Context, count int) (*llb.State, *docke
138138
}
139139
return &st, &img, nil
140140
case "git":
141-
st, ok := DetectGitContext(nc.input, true)
141+
st, ok, err := DetectGitContext(nc.input, true)
142142
if !ok {
143143
return nil, nil, errors.Errorf("invalid git context %s", nc.input)
144144
}
145+
if err != nil {
146+
return nil, nil, err
147+
}
145148
return st, nil, nil
146149
case "http", "https":
147-
st, ok := DetectGitContext(nc.input, true)
150+
st, ok, err := DetectGitContext(nc.input, true)
148151
if !ok {
149152
httpst := llb.HTTP(nc.input, llb.WithCustomName("[context "+nc.nameWithPlatform+"] "+nc.input))
150153
st = &httpst
151154
}
155+
if err != nil {
156+
return nil, nil, err
157+
}
152158
return st, nil, nil
153159
case "oci-layout":
154160
refSpec := strings.TrimPrefix(vv[1], "//")

0 commit comments

Comments
 (0)