git url: support CSV form

CSV parameters can be now specified after "##" in a git URL.
e.g.,
https://github.com/user/repo.git##tag=mytag,branch=main,commit=cafebab,subdir=/dir

tag, branch, and commit are exclusive.
The follow-up commit will allow specifying tag and commit together.

Fix issue 4905, but "source" in the original proposal was renamed to "subdir".

The documents will be added in
https://github.com/docker/docs/blob/main/content/manuals/build/concepts/context.md#url-fragments

Signed-off-by: Akihiro Suda <[email protected]>

Author: AkihiroSuda

frontend/dockerfile/dockerfile2llb/convert.go

@@ -1464,7 +1464,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 		if len(cfg.params.SourcePaths) != 1 {
 			return errors.New("checksum can't be specified for multiple sources")
 		}
-		if !isHTTPSource(cfg.params.SourcePaths[0]) {
+		ok, err := isHTTPSource(cfg.params.SourcePaths[0])
+		if err != nil {
+			return err
+		}
+		if !ok {
 			return errors.New("checksum can't be specified for non-HTTP(S) sources")
 		}
 	}
@@ -1523,77 +1527,83 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 			} else {
 				a = a.Copy(st, "/", dest, opts...)
 			}
-		} else if isHTTPSource(src) {
-			if !cfg.isAddCommand {
-				return errors.New("source can't be a URL for COPY")
+		} else {
+			isHTTPSourceV, err := isHTTPSource(src)
+			if err != nil {
+				return err
 			}
+			if isHTTPSourceV {
+				if !cfg.isAddCommand {
+					return errors.New("source can't be a URL for COPY")
+				}
 
-			// Resources from remote URLs are not decompressed.
-			// https://docs.docker.com/engine/reference/builder/#add
-			//
-			// Note: mixing up remote archives and local archives in a single ADD instruction
-			// would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717
-			u, err := url.Parse(src)
-			f := "__unnamed__"
-			if err == nil {
-				if base := path.Base(u.Path); base != "." && base != "/" {
-					f = base
+				// Resources from remote URLs are not decompressed.
+				// https://docs.docker.com/engine/reference/builder/#add
+				//
+				// Note: mixing up remote archives and local archives in a single ADD instruction
+				// would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717
+				u, err := url.Parse(src)
+				f := "__unnamed__"
+				if err == nil {
+					if base := path.Base(u.Path); base != "." && base != "/" {
+						f = base
+					}
 				}
-			}
 
-			st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(cfg.checksum), dfCmd(cfg.params))
+				st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(cfg.checksum), dfCmd(cfg.params))
 
-			opts := append([]llb.CopyOption{&llb.CopyInfo{
-				Mode:           chopt,
-				CreateDestPath: true,
-			}}, copyOpt...)
+				opts := append([]llb.CopyOption{&llb.CopyInfo{
+					Mode:           chopt,
+					CreateDestPath: true,
+				}}, copyOpt...)
 
-			if a == nil {
-				a = llb.Copy(st, f, dest, opts...)
-			} else {
-				a = a.Copy(st, f, dest, opts...)
-			}
-		} else {
-			validateCopySourcePath(src, &cfg)
-			var patterns []string
-			if cfg.parents {
-				// detect optional pivot point
-				parent, pattern, ok := strings.Cut(src, "/./")
-				if !ok {
-					pattern = src
-					src = "/"
+				if a == nil {
+					a = llb.Copy(st, f, dest, opts...)
 				} else {
-					src = parent
+					a = a.Copy(st, f, dest, opts...)
 				}
+			} else {
+				validateCopySourcePath(src, &cfg)
+				var patterns []string
+				if cfg.parents {
+					// detect optional pivot point
+					parent, pattern, ok := strings.Cut(src, "/./")
+					if !ok {
+						pattern = src
+						src = "/"
+					} else {
+						src = parent
+					}
+
+					pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false)
+					if err != nil {
+						return errors.Wrap(err, "removing drive letter")
+					}
 
-				pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false)
+					patterns = []string{strings.TrimPrefix(pattern, "/")}
+				}
+
+				src, err = system.NormalizePath("/", src, d.platform.OS, false)
 				if err != nil {
 					return errors.Wrap(err, "removing drive letter")
 				}
 
-				patterns = []string{strings.TrimPrefix(pattern, "/")}
-			}
-
-			src, err = system.NormalizePath("/", src, d.platform.OS, false)
-			if err != nil {
-				return errors.Wrap(err, "removing drive letter")
-			}
-
-			opts := append([]llb.CopyOption{&llb.CopyInfo{
-				Mode:                chopt,
-				FollowSymlinks:      true,
-				CopyDirContentsOnly: true,
-				IncludePatterns:     patterns,
-				AttemptUnpack:       cfg.isAddCommand,
-				CreateDestPath:      true,
-				AllowWildcard:       true,
-				AllowEmptyWildcard:  true,
-			}}, copyOpt...)
-
-			if a == nil {
-				a = llb.Copy(cfg.source, src, dest, opts...)
-			} else {
-				a = a.Copy(cfg.source, src, dest, opts...)
+				opts := append([]llb.CopyOption{&llb.CopyInfo{
+					Mode:                chopt,
+					FollowSymlinks:      true,
+					CopyDirContentsOnly: true,
+					IncludePatterns:     patterns,
+					AttemptUnpack:       cfg.isAddCommand,
+					CreateDestPath:      true,
+					AllowWildcard:       true,
+					AllowEmptyWildcard:  true,
+				}}, copyOpt...)
+
+				if a == nil {
+					a = llb.Copy(cfg.source, src, dest, opts...)
+				} else {
+					a = a.Copy(cfg.source, src, dest, opts...)
+				}
 			}
 		}
 	}
@@ -2255,15 +2265,22 @@ func commonImageNames() []string {
 	return out
 }
 
-func isHTTPSource(src string) bool {
+func isHTTPSource(src string) (bool, error) {
 	if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") {
-		return false
+		return false, nil
 	}
 	// https://github.com/ORG/REPO.git is a git source, not an http source
-	if gitRef, gitErr := gitutil.ParseGitRef(src); gitRef != nil && gitErr == nil {
-		return false
+	gitRef, gitErr := gitutil.ParseGitRef(src)
+	var eiuf *gitutil.ErrInvalidURLFragemnt
+	if errors.As(gitErr, &eiuf) {
+		// this is a git source, and it has an invalid URL fragment
+		// (e.g., both tag and branch are specified)
+		return false, gitErr
+	}
+	if gitRef != nil && gitErr == nil {
+		return false, nil
 	}
-	return true
+	return true, nil
 }
 
 func isEnabledForStage(stage string, value string) bool {

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) {

util/gitutil/git_ref_test.go

@@ -138,6 +138,15 @@ func TestParseGitRef(t *testing.T) {
 			ref:      ".git",
 			expected: nil,
 		},
+		{
+			ref: "https://example.com/foo.git##branch=release/1.2,subdir=/subdir",
+			expected: &GitRef{
+				Remote:    "https://example.com/foo.git",
+				ShortName: "foo",
+				Commit:    "release/1.2",
+				SubDir:    "/subdir",
+			},
+		},
 	}
 	for _, tt := range cases {
 		tt := tt

util/gitutil/git_url.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/moby/buildkit/util/sshutil"
 	"github.com/pkg/errors"
+	"github.com/tonistiigi/go-csvvalue"
 )
 
 const (
@@ -66,12 +67,59 @@ type GitURLFragment struct {
 
 // splitGitFragment splits a git URL fragment into its respective git
 // reference and subdirectory components.
-func splitGitFragment(fragment string) *GitURLFragment {
+func splitGitFragment(fragment string) (*GitURLFragment, error) {
+	if strings.HasPrefix(fragment, "#") {
+		// Double-hash in the unparsed URL.
+		// e.g., https://github.com/user/repo.git##tag=tag,subdir=/dir
+		return splitGitFragmentCSVForm(fragment)
+	}
+	// Single-hash in the unparsed URL.
+	// e.g., https://github.com/user/repo.git#branch_or_tag_or_commit:dir
 	if fragment == "" {
-		return nil
+		return nil, nil
 	}
 	ref, subdir, _ := strings.Cut(fragment, ":")
-	return &GitURLFragment{Ref: ref, Subdir: subdir}
+	return &GitURLFragment{Ref: ref, Subdir: subdir}, nil
+}
+
+func splitGitFragmentCSVForm(fragment string) (*GitURLFragment, error) {
+	fragment = strings.TrimPrefix(fragment, "#")
+	if fragment == "" {
+		return nil, nil
+	}
+	fields, err := csvvalue.Fields(fragment, nil)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to parse CSV %q", fragment)
+	}
+
+	res := &GitURLFragment{}
+	refs := make(map[string]string)
+	for _, field := range fields {
+		key, value, ok := strings.Cut(field, "=")
+		if !ok {
+			return nil, errors.Errorf("invalid field '%s' must be a key=value pair", field)
+		}
+		key = strings.ToLower(key)
+		switch key {
+		case "tag", "branch", "commit":
+			refs[key] = value
+		case "subdir":
+			res.Subdir = value
+		default:
+			return nil, errors.Errorf("unexpected key '%s' in '%s' (supported keys: tag, branch, commit, subdir)", key, field)
+		}
+	}
+	if len(refs) > 0 {
+		if len(refs) > 1 {
+			// TODO: allow specifying tag and commit together https://github.com/moby/buildkit/issues/5871
+			return nil, errors.New("tag, branch, and commit are exclusive")
+		}
+		for _, v := range refs {
+			res.Ref = v
+			break
+		}
+	}
+	return res, nil
 }
 
 // ParseURL parses a BuildKit-style Git URL (that may contain additional
@@ -86,11 +134,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 +153,46 @@ func IsGitTransport(remote string) bool {
 	return sshutil.IsImplicitSSHTransport(remote)
 }
 
-func fromURL(url *url.URL) *GitURL {
+// ErrInvalidURLFragemnt is returned for an invalid URL fragment
+type ErrInvalidURLFragemnt struct {
+	error
+}
+
+func fromURL(url *url.URL) (*GitURL, error) {
 	withoutFragment := *url
 	withoutFragment.Fragment = ""
-	return &GitURL{
+	fragment, err := splitGitFragment(url.Fragment)
+	if err != nil {
+		return nil, &ErrInvalidURLFragemnt{
+			error: errors.Wrapf(err, "failed to parse URL fragment %q", url.Fragment),
+		}
+	}
+	gitURL := &GitURL{
 		Scheme:   url.Scheme,
 		User:     url.User,
 		Host:     url.Host,
 		Path:     url.Path,
-		Fragment: splitGitFragment(url.Fragment),
+		Fragment: fragment,
 		Remote:   withoutFragment.String(),
 	}
+	return gitURL, nil
 }
 
-func fromSCPStyleURL(url *sshutil.SCPStyleURL) *GitURL {
+func fromSCPStyleURL(url *sshutil.SCPStyleURL) (*GitURL, error) {
 	withoutFragment := *url
 	withoutFragment.Fragment = ""
+	fragment, err := splitGitFragment(url.Fragment)
+	if err != nil {
+		return nil, &ErrInvalidURLFragemnt{
+			error: errors.Wrapf(err, "failed to parse URL fragment %q", url.Fragment),
+		}
+	}
 	return &GitURL{
 		Scheme:   SSHProtocol,
 		User:     url.User,
 		Host:     url.Host,
 		Path:     url.Path,
-		Fragment: splitGitFragment(url.Fragment),
+		Fragment: fragment,
 		Remote:   withoutFragment.String(),
-	}
+	}, nil
 }