Merge pull request #6172 from tonistiigi/git-querystring

Support querystring form Git URLs

Author: AkihiroSuda

client/llb/git_test.go

@@ -0,0 +1,94 @@
+package llb
+
+import (
+	"context"
+	"testing"
+
+	"github.com/moby/buildkit/solver/pb"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGit(t *testing.T) {
+	t.Parallel()
+
+	type tcase struct {
+		name       string
+		st         State
+		identifier string
+		attrs      map[string]string
+	}
+
+	tcases := []tcase{
+		{
+			name:       "refarg",
+			st:         Git("github.com/foo/bar.git", "ref"),
+			identifier: "git://github.com/foo/bar.git#ref",
+			attrs: map[string]string{
+				"git.authheadersecret": "GIT_AUTH_HEADER",
+				"git.authtokensecret":  "GIT_AUTH_TOKEN",
+				"git.fullurl":          "https://github.com/foo/bar.git",
+			},
+		},
+		{
+			name:       "refarg with subdir",
+			st:         Git("github.com/foo/bar.git", "ref:subdir"),
+			identifier: "git://github.com/foo/bar.git#ref:subdir",
+			attrs: map[string]string{
+				"git.authheadersecret": "GIT_AUTH_HEADER",
+				"git.authtokensecret":  "GIT_AUTH_TOKEN",
+				"git.fullurl":          "https://github.com/foo/bar.git",
+			},
+		},
+		{
+			name:       "refarg with subdir func",
+			st:         Git("github.com/foo/bar.git", "ref", GitSubDir("subdir")),
+			identifier: "git://github.com/foo/bar.git#ref:subdir",
+			attrs: map[string]string{
+				"git.authheadersecret": "GIT_AUTH_HEADER",
+				"git.authtokensecret":  "GIT_AUTH_TOKEN",
+				"git.fullurl":          "https://github.com/foo/bar.git",
+			},
+		},
+		{
+			name:       "refarg with override",
+			st:         Git("github.com/foo/bar.git", "ref:dir", GitRef("v1.0")),
+			identifier: "git://github.com/foo/bar.git#v1.0:dir",
+			attrs: map[string]string{
+				"git.authheadersecret": "GIT_AUTH_HEADER",
+				"git.authtokensecret":  "GIT_AUTH_TOKEN",
+				"git.fullurl":          "https://github.com/foo/bar.git",
+			},
+		},
+		{
+			name:       "funcs only",
+			st:         Git("github.com/foo/bar.git", "", GitRef("v1.0"), GitSubDir("dir")),
+			identifier: "git://github.com/foo/bar.git#v1.0:dir",
+			attrs: map[string]string{
+				"git.authheadersecret": "GIT_AUTH_HEADER",
+				"git.authtokensecret":  "GIT_AUTH_TOKEN",
+				"git.fullurl":          "https://github.com/foo/bar.git",
+			},
+		},
+	}
+
+	for _, tc := range tcases {
+		t.Run(tc.name, func(t *testing.T) {
+			st := tc.st
+			def, err := st.Marshal(context.TODO())
+
+			require.NoError(t, err)
+
+			m, arr := parseDef(t, def.Def)
+			require.Equal(t, 2, len(arr))
+
+			dgst, idx := last(t, arr)
+			require.Equal(t, 0, idx)
+			require.Equal(t, m[dgst], arr[0])
+
+			g := arr[0].Op.(*pb.Op_Source).Source
+
+			require.Equal(t, tc.identifier, g.Identifier)
+			require.Equal(t, tc.attrs, g.Attrs)
+		})
+	}
+}

client/llb/source.go

@@ -247,9 +247,13 @@ const (
 // Formats that utilize SSH may need to supply credentials as a [GitOption].
 // You may need to check the source code for a full list of supported formats.
 //
+// Fragment can be used to pass ref:subdir format that can set in (old-style)
+// Docker Git URL format after # . This is provided for backwards compatibility.
+// It is recommended to leave it empty and call GitRef(), GitSubdir() options instead.
+//
 // By default the git repository is cloned with `--depth=1` to reduce the amount of data downloaded.
 // Additionally the ".git" directory is removed after the clone, you can keep ith with the [KeepGitDir] [GitOption].
-func Git(url, ref string, opts ...GitOption) State {
+func Git(url, fragment string, opts ...GitOption) State {
 	remote, err := gitutil.ParseURL(url)
 	if errors.Is(err, gitutil.ErrUnknownProtocol) {
 		url = "https://" + url
@@ -259,6 +263,20 @@ func Git(url, ref string, opts ...GitOption) State {
 		url = remote.Remote
 	}
 
+	gi := &GitInfo{
+		AuthHeaderSecret: GitAuthHeaderKey,
+		AuthTokenSecret:  GitAuthTokenKey,
+	}
+	ref, subdir, ok := strings.Cut(fragment, ":")
+	if ref != "" {
+		GitRef(ref).SetGitOption(gi)
+	}
+	if ok && subdir != "" {
+		GitSubDir(subdir).SetGitOption(gi)
+	}
+	for _, o := range opts {
+		o.SetGitOption(gi)
+	}
 	var id string
 	if err != nil {
 		// 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 {
 		// for different protocols (e.g. https and ssh) that have the same
 		// host/path/fragment combination.
 		id = remote.Host + path.Join("/", remote.Path)
-		if ref != "" {
-			id += "#" + ref
+		if gi.Ref != "" || gi.SubDir != "" {
+			id += "#" + gi.Ref
+			if gi.SubDir != "" {
+				id += ":" + gi.SubDir
+			}
 		}
 	}
-
-	gi := &GitInfo{
-		AuthHeaderSecret: GitAuthHeaderKey,
-		AuthTokenSecret:  GitAuthTokenKey,
-	}
-	for _, o := range opts {
-		o.SetGitOption(gi)
-	}
 	attrs := map[string]string{}
 	if gi.KeepGitDir {
 		attrs[pb.AttrKeepGitDir] = "true"
@@ -352,6 +365,20 @@ type GitInfo struct {
 	KnownSSHHosts    string
 	MountSSHSock     string
 	Checksum         string
+	Ref              string
+	SubDir           string
+}
+
+func GitRef(v string) GitOption {
+	return gitOptionFunc(func(gi *GitInfo) {
+		gi.Ref = v
+	})
+}
+
+func GitSubDir(v string) GitOption {
+	return gitOptionFunc(func(gi *GitInfo) {
+		gi.SubDir = v
+	})
 }
 
 func KeepGitDir() GitOption {

frontend/dockerfile/dfgitutil/git_ref.go

@@ -23,9 +23,12 @@ type GitRef struct {
 	// e.g., "bar" for "https://github.com/foo/bar.git"
 	ShortName string
 
-	// Commit is a commit hash, a tag, or branch name.
-	// Commit is optional.
-	Commit string
+	// Ref is a commit hash, a tag, or branch name.
+	// Ref is optional.
+	Ref string
+
+	// Checksum is a commit hash.
+	Checksum string
 
 	// SubDir is a directory path inside the repo.
 	// SubDir is optional.
@@ -48,10 +51,8 @@ type GitRef struct {
 	UnencryptedTCP bool
 }
 
-// var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
-
 // ParseGitRef parses a git ref.
-func ParseGitRef(ref string) (*GitRef, error) {
+func ParseGitRef(ref string) (*GitRef, bool, error) {
 	res := &GitRef{}
 
 	var (
@@ -60,21 +61,25 @@ func ParseGitRef(ref string) (*GitRef, error) {
 	)
 
 	if strings.HasPrefix(ref, "./") || strings.HasPrefix(ref, "../") {
-		return nil, cerrdefs.ErrInvalidArgument
+		return nil, false, errors.WithStack(cerrdefs.ErrInvalidArgument)
 	} else if strings.HasPrefix(ref, "github.com/") {
 		res.IndistinguishableFromLocal = true // Deprecated
-		remote = gitutil.FromURL(&url.URL{
-			Scheme: "https",
-			Host:   "github.com",
-			Path:   strings.TrimPrefix(ref, "github.com/"),
-		})
+		u, err := url.Parse(ref)
+		if err != nil {
+			return nil, false, err
+		}
+		u.Scheme = "https"
+		remote, err = gitutil.FromURL(u)
+		if err != nil {
+			return nil, false, err
+		}
 	} else {
 		remote, err = gitutil.ParseURL(ref)
 		if errors.Is(err, gitutil.ErrUnknownProtocol) {
-			return nil, err
+			return nil, false, err
 		}
 		if err != nil {
-			return nil, err
+			return nil, false, err
 		}
 
 		switch remote.Scheme {
@@ -86,7 +91,7 @@ func ParseGitRef(ref string) (*GitRef, error) {
 		// An HTTP(S) URL is considered to be a valid git ref only when it has the ".git[...]" suffix.
 		case gitutil.HTTPProtocol, gitutil.HTTPSProtocol:
 			if !strings.HasSuffix(remote.Path, ".git") {
-				return nil, cerrdefs.ErrInvalidArgument
+				return nil, false, errors.WithStack(cerrdefs.ErrInvalidArgument)
 			}
 		}
 	}
@@ -96,11 +101,83 @@ func ParseGitRef(ref string) (*GitRef, error) {
 		_, res.Remote, _ = strings.Cut(res.Remote, "://")
 	}
 	if remote.Opts != nil {
-		res.Commit, res.SubDir = remote.Opts.Ref, remote.Opts.Subdir
+		res.Ref, res.SubDir = remote.Opts.Ref, remote.Opts.Subdir
 	}
 
 	repoSplitBySlash := strings.Split(res.Remote, "/")
 	res.ShortName = strings.TrimSuffix(repoSplitBySlash[len(repoSplitBySlash)-1], ".git")
 
-	return res, nil
+	if err := res.loadQuery(remote.Query); err != nil {
+		return nil, true, err
+	}
+
+	return res, true, nil
+}
+
+func (gf *GitRef) loadQuery(query url.Values) error {
+	if len(query) == 0 {
+		return nil
+	}
+	var tag, branch string
+	for k, v := range query {
+		switch len(v) {
+		case 0:
+			return errors.Errorf("query %q has no value", k)
+		case 1:
+			if v[0] == "" {
+				return errors.Errorf("query %q has no value", k)
+			}
+			// NOP
+		default:
+			return errors.Errorf("query %q has multiple values", k)
+		}
+		switch k {
+		case "ref":
+			if gf.Ref != "" && gf.Ref != v[0] {
+				return errors.Errorf("ref conflicts: %q vs %q", gf.Ref, v[0])
+			}
+			gf.Ref = v[0]
+		case "tag":
+			tag = v[0]
+		case "branch":
+			branch = v[0]
+		case "subdir":
+			if gf.SubDir != "" && gf.SubDir != v[0] {
+				return errors.Errorf("subdir conflicts: %q vs %q", gf.SubDir, v[0])
+			}
+			gf.SubDir = v[0]
+		case "checksum", "commit":
+			gf.Checksum = v[0]
+		default:
+			return errors.Errorf("unexpected query %q", k)
+		}
+	}
+	if tag != "" {
+		const tagPrefix = "refs/tags/"
+		if !strings.HasPrefix(tag, tagPrefix) {
+			tag = tagPrefix + tag
+		}
+		if gf.Ref != "" && gf.Ref != tag {
+			return errors.Errorf("ref conflicts: %q vs %q", gf.Ref, tag)
+		}
+		gf.Ref = tag
+	}
+	if branch != "" {
+		if tag != "" {
+			// TODO: consider allowing this, when the tag actually exists on the branch
+			return errors.New("branch conflicts with tag")
+		}
+		const branchPrefix = "refs/heads/"
+		if !strings.HasPrefix(branch, branchPrefix) {
+			branch = branchPrefix + branch
+		}
+		if gf.Ref != "" && gf.Ref != branch {
+			return errors.Errorf("ref conflicts: %q vs %q", gf.Ref, branch)
+		}
+		gf.Ref = branch
+	}
+	if gf.Checksum != "" && gf.Ref == "" {
+		gf.Ref = gf.Checksum
+	}
+	return nil
 }