[0.10] git context: fix support for empty git ref with subdir

frontend/dockerfile/builder/build.go

@@ -28,6 +28,7 @@ import (
 	"github.com/moby/buildkit/solver/pb"
 	"github.com/moby/buildkit/util/apicaps"
 	binfotypes "github.com/moby/buildkit/util/buildinfo/types"
+	"github.com/moby/buildkit/util/gitutil"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"
@@ -71,7 +72,6 @@ const (
 )
 
 var httpPrefix = regexp.MustCompile(`^https?://`)
-var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
 
 func Build(ctx context.Context, c client.Client) (*client.Result, error) {
 	opts := c.BuildOpts().Opts
@@ -172,7 +172,11 @@ func Build(ctx context.Context, c client.Client) (*client.Result, error) {
 
 	var buildContext *llb.State
 	isNotLocalContext := false
-	if st, ok := detectGitContext(opts[localNameContext], opts[keyContextKeepGitDirArg]); ok {
+	keepGit := false
+	if v, err := strconv.ParseBool(opts[keyContextKeepGitDirArg]); err == nil {
+		keepGit = v
+	}
+	if st, ok := detectGitContext(opts[localNameContext], keepGit); ok {
 		if !forceLocalDockerfile {
 			src = *st
 		}
@@ -606,40 +610,21 @@ func filter(opt map[string]string, key string) map[string]string {
 	return m
 }
 
-func detectGitContext(ref, gitContext string) (*llb.State, bool) {
-	found := false
-	if httpPrefix.MatchString(ref) && gitURLPathWithFragmentSuffix.MatchString(ref) {
-		found = true
-	}
-
-	keepGit := false
-	if gitContext != "" {
-		if v, err := strconv.ParseBool(gitContext); err == nil {
-			keepGit = v
-		}
-	}
-
-	for _, prefix := range []string{"git://", "github.com/", "git@"} {
-		if strings.HasPrefix(ref, prefix) {
-			found = true
-			break
-		}
-	}
-	if !found {
+func detectGitContext(ref string, keepGit bool) (*llb.State, bool) {
+	g, err := gitutil.ParseGitRef(ref)
+	if err != nil {
 		return nil, false
 	}
-
-	parts := strings.SplitN(ref, "#", 2)
-	branch := ""
-	if len(parts) > 1 {
-		branch = parts[1]
+	commit := g.Commit
+	if g.SubDir != "" {
+		commit += ":" + g.SubDir
 	}
 	gitOpts := []llb.GitOption{dockerfile2llb.WithInternalName("load git source " + ref)}
 	if keepGit {
 		gitOpts = append(gitOpts, llb.KeepGitDir())
 	}
 
-	st := llb.Git(parts[0], branch, gitOpts...)
+	st := llb.Git(g.Remote, commit, gitOpts...)
 	return &st, true
 }
 
@@ -888,13 +873,13 @@ func contextByName(ctx context.Context, c client.Client, name string, platform *
 		}
 		return &st, &img, nil, nil
 	case "git":
-		st, ok := detectGitContext(v, "1")
+		st, ok := detectGitContext(v, true)
 		if !ok {
 			return nil, nil, nil, errors.Errorf("invalid git context %s", v)
 		}
 		return st, nil, nil, nil
 	case "http", "https":
-		st, ok := detectGitContext(v, "1")
+		st, ok := detectGitContext(v, true)
 		if !ok {
 			httpst := llb.HTTP(v, llb.WithCustomName("[context "+name+"] "+v))
 			st = &httpst

util/gitutil/git_ref.go

@@ -0,0 +1,85 @@
+package gitutil
+
+import (
+	"regexp"
+	"strings"
+
+	"github.com/containerd/containerd/errdefs"
+)
+
+// GitRef represents a git ref.
+//
+// Examples:
+// - "https://github.com/foo/bar.git#baz/qux:quux/quuz" is parsed into:
+//   {Remote: "https://github.com/foo/bar.git", ShortName: "bar", Commit:"baz/qux", SubDir: "quux/quuz"}.
+type GitRef struct {
+	// Remote is the remote repository path.
+	Remote string
+
+	// ShortName is the directory name of the repo.
+	// 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
+
+	// SubDir is a directory path inside the repo.
+	// SubDir is optional.
+	SubDir string
+
+	// IndistinguishableFromLocal is true for a ref that is indistinguishable from a local file path,
+	// e.g., "github.com/foo/bar".
+	//
+	// Deprecated.
+	// Instead, use a distinguishable form such as "https://github.com/foo/bar.git".
+	//
+	// The dockerfile frontend still accepts this form only for build contexts.
+	IndistinguishableFromLocal bool
+
+	// UnencryptedTCP is true for a ref that needs an unencrypted TCP connection,
+	// e.g., "git://..." and "http://..." .
+	//
+	// Discouraged, although not deprecated.
+	// Instead, consider using an encrypted TCP connection such as "git@github.com/foo/bar.git" or "https://github.com/foo/bar.git".
+	UnencryptedTCP bool
+}
+
+// var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
+
+// ParseGitRef parses a git ref.
+func ParseGitRef(ref string) (*GitRef, error) {
+	res := &GitRef{}
+
+	if strings.HasPrefix(ref, "github.com/") {
+		res.IndistinguishableFromLocal = true // Deprecated
+	} else {
+		_, proto := ParseProtocol(ref)
+		switch proto {
+		case UnknownProtocol:
+			return nil, errdefs.ErrInvalidArgument
+		}
+		switch proto {
+		case HTTPProtocol, GitProtocol:
+			res.UnencryptedTCP = true // Discouraged, but not deprecated
+		}
+		switch proto {
+		// An HTTP(S) URL is considered to be a valid git ref only when it has the ".git[...]" suffix.
+		case HTTPProtocol, HTTPSProtocol:
+			var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
+			if !gitURLPathWithFragmentSuffix.MatchString(ref) {
+				return nil, errdefs.ErrInvalidArgument
+			}
+		}
+	}
+
+	var fragment string
+	res.Remote, fragment, _ = strings.Cut(ref, "#")
+	if len(res.Remote) == 0 {
+		return res, errdefs.ErrInvalidArgument
+	}
+	res.Commit, res.SubDir, _ = strings.Cut(fragment, ":")
+	repoSplitBySlash := strings.Split(res.Remote, "/")
+	res.ShortName = strings.TrimSuffix(repoSplitBySlash[len(repoSplitBySlash)-1], ".git")
+	return res, nil
+}

util/gitutil/git_ref_test.go

@@ -0,0 +1,140 @@
+package gitutil
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestParseGitRef(t *testing.T) {
+	cases := []struct {
+		ref      string
+		expected *GitRef
+	}{
+		{
+			ref:      "https://example.com/",
+			expected: nil,
+		},
+		{
+			ref:      "https://example.com/foo",
+			expected: nil,
+		},
+		{
+			ref: "https://example.com/foo.git",
+			expected: &GitRef{
+				Remote:    "https://example.com/foo.git",
+				ShortName: "foo",
+			},
+		},
+		{
+			ref: "https://example.com/foo.git#deadbeef",
+			expected: &GitRef{
+				Remote:    "https://example.com/foo.git",
+				ShortName: "foo",
+				Commit:    "deadbeef",
+			},
+		},
+		{
+			ref: "https://example.com/foo.git#release/1.2",
+			expected: &GitRef{
+				Remote:    "https://example.com/foo.git",
+				ShortName: "foo",
+				Commit:    "release/1.2",
+			},
+		},
+		{
+			ref:      "https://example.com/foo.git/",
+			expected: nil,
+		},
+		{
+			ref:      "https://example.com/foo.git.bar",
+			expected: nil,
+		},
+		{
+			ref: "git://example.com/foo",
+			expected: &GitRef{
+				Remote:         "git://example.com/foo",
+				ShortName:      "foo",
+				UnencryptedTCP: true,
+			},
+		},
+		{
+			ref: "github.com/moby/buildkit",
+			expected: &GitRef{
+				Remote:                     "github.com/moby/buildkit",
+				ShortName:                  "buildkit",
+				IndistinguishableFromLocal: true,
+			},
+		},
+		{
+			ref:      "https://github.com/moby/buildkit",
+			expected: nil,
+		},
+		{
+			ref: "https://github.com/moby/buildkit.git",
+			expected: &GitRef{
+				Remote:    "https://github.com/moby/buildkit.git",
+				ShortName: "buildkit",
+			},
+		},
+		{
+			ref: "git@github.com:moby/buildkit",
+			expected: &GitRef{
+				Remote:    "git@github.com:moby/buildkit",
+				ShortName: "buildkit",
+			},
+		},
+		{
+			ref: "git@github.com:moby/buildkit.git",
+			expected: &GitRef{
+				Remote:    "git@github.com:moby/buildkit.git",
+				ShortName: "buildkit",
+			},
+		},
+		{
+			ref: "git@bitbucket.org:atlassianlabs/atlassian-docker.git",
+			expected: &GitRef{
+				Remote:    "git@bitbucket.org:atlassianlabs/atlassian-docker.git",
+				ShortName: "atlassian-docker",
+			},
+		},
+		{
+			ref: "https://github.com/foo/bar.git#baz/qux:quux/quuz",
+			expected: &GitRef{
+				Remote:    "https://github.com/foo/bar.git",
+				ShortName: "bar",
+				Commit:    "baz/qux",
+				SubDir:    "quux/quuz",
+			},
+		},
+		{
+			ref:      "https://github.com/docker/docker.git:#branch",
+			expected: nil,
+		},
+		{
+			ref: "https://github.com/docker/docker.git#:myfolder",
+			expected: &GitRef{
+				Remote:    "https://github.com/docker/docker.git",
+				ShortName: "docker",
+				SubDir:    "myfolder",
+			},
+		},
+	}
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.ref, func(t *testing.T) {
+			got, err := ParseGitRef(tt.ref)
+			if tt.expected == nil {
+				if err == nil {
+					t.Errorf("expected an error for ParseGitRef(%q)", tt.ref)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("got an unexpected error: ParseGitRef(%q): %v", tt.ref, err)
+				}
+				if !reflect.DeepEqual(got, tt.expected) {
+					t.Errorf("expected ParseGitRef(%q) to return %#v, got %#v", tt.ref, tt.expected, got)
+				}
+			}
+		})
+	}
+}