From 75e248d497ea18e27c81df2bf9e74fb6cd5ba5d9 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Thu, 15 May 2025 18:34:34 +0900 Subject: [PATCH] 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 --- util/gitutil/git_ref.go | 5 +- util/gitutil/git_ref_test.go | 9 ++++ util/gitutil/git_url.go | 89 ++++++++++++++++++++++++++++++------ util/gitutil/git_url_test.go | 59 ++++++++++++++++++++++++ util/sshutil/scpurl.go | 34 ++++++++++---- 5 files changed, 173 insertions(+), 23 deletions(-) diff --git a/util/gitutil/git_ref.go b/util/gitutil/git_ref.go index cb562acf5481..d6eae2b92f1f 100644 --- a/util/gitutil/git_ref.go +++ b/util/gitutil/git_ref.go @@ -61,11 +61,14 @@ func ParseGitRef(ref string) (*GitRef, error) { return nil, cerrdefs.ErrInvalidArgument } else if strings.HasPrefix(ref, "github.com/") { res.IndistinguishableFromLocal = true // Deprecated - remote = fromURL(&url.URL{ + remote, err = fromURL(&url.URL{ Scheme: "https", Host: "github.com", Path: strings.TrimPrefix(ref, "github.com/"), }) + if err != nil { + return nil, err + } } else { remote, err = ParseURL(ref) if errors.Is(err, ErrUnknownProtocol) { diff --git a/util/gitutil/git_ref_test.go b/util/gitutil/git_ref_test.go index d470a09fc5d0..c0143ba0a513 100644 --- a/util/gitutil/git_ref_test.go +++ b/util/gitutil/git_ref_test.go @@ -138,6 +138,15 @@ func TestParseGitRef(t *testing.T) { ref: ".git", expected: nil, }, + { + ref: "https://github.com/docker/docker.git?ref=v1.0.0&subdir=/subdir", + expected: &GitRef{ + Remote: "https://github.com/docker/docker.git", + ShortName: "docker", + Commit: "v1.0.0", + SubDir: "/subdir", + }, + }, } for _, tt := range cases { t.Run(tt.ref, func(t *testing.T) { diff --git a/util/gitutil/git_url.go b/util/gitutil/git_url.go index 338893b279e1..e2f080ba9500 100644 --- a/util/gitutil/git_url.go +++ b/util/gitutil/git_url.go @@ -56,7 +56,7 @@ type GitURL struct { } // GitURLOpts is the buildkit-specific metadata extracted from the fragment -// of a remote URL. +// or the query of a remote URL. type GitURLOpts struct { // Ref is the git reference Ref string @@ -66,12 +66,63 @@ type GitURLOpts struct { // parseOpts splits a git URL fragment into its respective git // reference and subdirectory components. -func parseOpts(fragment string) *GitURLOpts { - if fragment == "" { - return nil +func parseOpts(fragment string, query url.Values) (*GitURLOpts, error) { + if fragment == "" && len(query) == 0 { + return nil, nil } - ref, subdir, _ := strings.Cut(fragment, ":") - return &GitURLOpts{Ref: ref, Subdir: subdir} + opts := &GitURLOpts{} + if fragment != "" { + opts.Ref, opts.Subdir, _ = strings.Cut(fragment, ":") + } + var tag, branch string + for k, v := range query { + switch len(v) { + case 0: + return nil, errors.Errorf("query %q has no value", k) + case 1: + if v[0] == "" { + return nil, errors.Errorf("query %q has no value", k) + } + // NOP + default: + return nil, errors.Errorf("query %q has multiple values", k) + } + switch k { + case "ref": + if opts.Ref != "" && opts.Ref != v[0] { + return nil, errors.Errorf("ref conflicts: %q vs %q", opts.Ref, v[0]) + } + opts.Ref = v[0] + case "tag": + tag = v[0] + case "branch": + branch = v[0] + case "subdir": + if opts.Subdir != "" && opts.Subdir != v[0] { + return nil, errors.Errorf("subdir conflicts: %q vs %q", opts.Subdir, v[0]) + } + opts.Subdir = v[0] + default: + return nil, errors.Errorf("unexpected query %q", k) + } + } + if tag != "" { + if opts.Ref != "" { + return nil, errors.New("tag conflicts with ref") + } + opts.Ref = "refs/tags/" + tag + } + if branch != "" { + if tag != "" { + // TODO: consider allowing this, when the tag actually exists on the branch + return nil, errors.New("branch conflicts with tag") + } + if opts.Ref != "" { + return nil, errors.New("branch conflicts with ref") + } + opts.Ref = "refs/heads/" + branch + } + return opts, nil } // ParseURL parses a BuildKit-style Git URL (that may contain additional @@ -86,11 +137,11 @@ func ParseURL(remote string) (*GitURL, error) { if err != nil { return nil, err } - return fromURL(url), nil + return fromURL(url) } if url, err := sshutil.ParseSCPStyleURL(remote); err == nil { - return fromSCPStyleURL(url), nil + return fromSCPStyleURL(url) } return nil, ErrUnknownProtocol @@ -105,28 +156,38 @@ func IsGitTransport(remote string) bool { return sshutil.IsImplicitSSHTransport(remote) } -func fromURL(url *url.URL) *GitURL { +func fromURL(url *url.URL) (*GitURL, error) { withoutOpts := *url withoutOpts.Fragment = "" + withoutOpts.RawQuery = "" + opts, err := parseOpts(url.Fragment, url.Query()) + if err != nil { + return nil, err + } return &GitURL{ Scheme: url.Scheme, User: url.User, Host: url.Host, Path: url.Path, - Opts: parseOpts(url.Fragment), + Opts: opts, Remote: withoutOpts.String(), - } + }, nil } -func fromSCPStyleURL(url *sshutil.SCPStyleURL) *GitURL { +func fromSCPStyleURL(url *sshutil.SCPStyleURL) (*GitURL, error) { withoutOpts := *url withoutOpts.Fragment = "" + withoutOpts.Query = nil + opts, err := parseOpts(url.Fragment, url.Query) + if err != nil { + return nil, err + } return &GitURL{ Scheme: SSHProtocol, User: url.User, Host: url.Host, Path: url.Path, - Opts: parseOpts(url.Fragment), + Opts: opts, Remote: withoutOpts.String(), - } + }, nil } diff --git a/util/gitutil/git_url_test.go b/util/gitutil/git_url_test.go index 3306b06f7bd2..be3409a413ea 100644 --- a/util/gitutil/git_url_test.go +++ b/util/gitutil/git_url_test.go @@ -151,6 +151,65 @@ func TestParseURL(t *testing.T) { Path: "/moby/buildkit", }, }, + { + url: "https://github.com/moby/buildkit?ref=v1.0.0&subdir=/subdir", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Opts: &GitURLOpts{Ref: "v1.0.0", Subdir: "/subdir"}, + }, + }, + { + url: "https://github.com/moby/buildkit?subdir=/subdir#v1.0.0", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Opts: &GitURLOpts{Ref: "v1.0.0", Subdir: "/subdir"}, + }, + }, + { + url: "https://github.com/moby/buildkit?tag=v1.0.0", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Opts: &GitURLOpts{Ref: "refs/tags/v1.0.0"}, + }, + }, + { + url: "https://github.com/moby/buildkit?branch=v1.0", + result: GitURL{ + Scheme: HTTPSProtocol, + Host: "github.com", + Path: "/moby/buildkit", + Opts: &GitURLOpts{Ref: "refs/heads/v1.0"}, + }, + }, + { + url: "https://github.com/moby/buildkit?ref=v1.0.0#v1.2.3", + err: true, + }, + { + url: "https://github.com/moby/buildkit?ref=v1.0.0&tag=v1.2.3", + err: true, + }, + { + // TODO: consider allowing this, when the tag actually exists on the branch + url: "https://github.com/moby/buildkit?tag=v1.0.0&branch=v1.0", + err: true, + }, + { + url: "git@github.com:moby/buildkit.git?subdir=/subdir#v1.0.0", + result: GitURL{ + Scheme: SSHProtocol, + Host: "github.com", + Path: "moby/buildkit.git", + User: url.User("git"), + Opts: &GitURLOpts{Ref: "v1.0.0", Subdir: "/subdir"}, + }, + }, } for _, test := range tests { t.Run(test.url, func(t *testing.T) { diff --git a/util/sshutil/scpurl.go b/util/sshutil/scpurl.go index 10491f32f08e..5d85f09a0ba6 100644 --- a/util/sshutil/scpurl.go +++ b/util/sshutil/scpurl.go @@ -1,13 +1,14 @@ package sshutil import ( - "errors" "fmt" "net/url" "regexp" + + "github.com/pkg/errors" ) -var gitSSHRegex = regexp.MustCompile("^([a-zA-Z0-9-_]+)@([a-zA-Z0-9-.]+):(.*?)(?:#(.*))?$") +var gitSSHRegex = regexp.MustCompile(`^([a-zA-Z0-9-_]+)@([a-zA-Z0-9-.]+):(.*?)(?:\?(.*?))?(?:#(.*))?$`) func IsImplicitSSHTransport(s string) bool { return gitSSHRegex.MatchString(s) @@ -18,6 +19,7 @@ type SCPStyleURL struct { Host string Path string + Query url.Values Fragment string } @@ -26,18 +28,34 @@ func ParseSCPStyleURL(raw string) (*SCPStyleURL, error) { if matches == nil { return nil, errors.New("invalid scp-style url") } + + rawQuery := matches[4] + vals := url.Values{} + if rawQuery != "" { + var err error + vals, err = url.ParseQuery(rawQuery) + if err != nil { + return nil, errors.Wrap(err, "invalid query in scp-style url") + } + } + return &SCPStyleURL{ User: url.User(matches[1]), Host: matches[2], Path: matches[3], - Fragment: matches[4], + Query: vals, + Fragment: matches[5], }, nil } -func (url *SCPStyleURL) String() string { - base := fmt.Sprintf("%s@%s:%s", url.User.String(), url.Host, url.Path) - if url.Fragment == "" { - return base +func (u *SCPStyleURL) String() string { + s := fmt.Sprintf("%s@%s:%s", u.User.String(), u.Host, u.Path) + + if len(u.Query) > 0 { + s += "?" + u.Query.Encode() + } + if u.Fragment != "" { + s += "#" + u.Fragment } - return base + "#" + url.Fragment + return s }