Merge pull request #40178 from thaJeztah/remote_context_refactor

builder/remotecontext: small refactor

builder/remotecontext/git/gitutils.go

@@ -10,7 +10,6 @@ import (
 	"strings"
 
 	"github.com/docker/docker/pkg/symlink"
-	"github.com/docker/docker/pkg/urlutil"
 	"github.com/pkg/errors"
 )
 
@@ -135,7 +134,7 @@ func fetchArgs(remoteURL string, ref string) []string {
 // Check if a given git URL supports a shallow git clone,
 // i.e. it is a non-HTTP server or a smart HTTP server.
 func supportsShallowClone(remoteURL string) bool {
-	if urlutil.IsURL(remoteURL) {
+	if scheme := getScheme(remoteURL); scheme == "http" || scheme == "https" {
 		// Check if the HTTP server is smart
 
 		// Smart servers must correctly respond to a query for the git-upload-pack service
@@ -205,5 +204,24 @@ func git(args ...string) ([]byte, error) {
 // isGitTransport returns true if the provided str is a git transport by inspecting
 // the prefix of the string for known protocols used in git.
 func isGitTransport(str string) bool {
-	return urlutil.IsURL(str) || strings.HasPrefix(str, "git://") || strings.HasPrefix(str, "git@")
+	if strings.HasPrefix(str, "git@") {
+		return true
+	}
+
+	switch getScheme(str) {
+	case "git", "http", "https":
+		return true
+	}
+
+	return false
+}
+
+// getScheme returns addresses' scheme in lowercase, or an empty
+// string in case address is an invalid URL.
+func getScheme(address string) string {
+	u, err := url.Parse(address)
+	if err != nil {
+		return ""
+	}
+	return u.Scheme
 }

builder/remotecontext/git/gitutils_test.go

@@ -19,32 +19,81 @@ import (
 )
 
 func TestParseRemoteURL(t *testing.T) {
-	dir, err := parseRemoteURL("git://github.com/user/repo.git")
+	tests := []struct {
-	assert.NilError(t, err)
+		doc      string
-	assert.Check(t, is.DeepEqual(gitRepo{"git://github.com/user/repo.git", "master", ""}, dir, cmpGitRepoOpt))
+		url      string
-
+		expected gitRepo
-	dir, err = parseRemoteURL("git://github.com/user/repo.git#mybranch:mydir/mysubdir/")
+	}{
-	assert.NilError(t, err)
+		{
-	assert.Check(t, is.DeepEqual(gitRepo{"git://github.com/user/repo.git", "mybranch", "mydir/mysubdir/"}, dir, cmpGitRepoOpt))
+			doc: "git scheme uppercase, no url-fragment",
-
+			url: "GIT://github.com/user/repo.git",
-	dir, err = parseRemoteURL("https://github.com/user/repo.git")
+			expected: gitRepo{
-	assert.NilError(t, err)
+				remote: "git://github.com/user/repo.git",
-	assert.Check(t, is.DeepEqual(gitRepo{"https://github.com/user/repo.git", "master", ""}, dir, cmpGitRepoOpt))
+				ref:    "master",
-
+			},
-	dir, err = parseRemoteURL("https://github.com/user/repo.git#mybranch:mydir/mysubdir/")
+		},
-	assert.NilError(t, err)
+		{
-	assert.Check(t, is.DeepEqual(gitRepo{"https://github.com/user/repo.git", "mybranch", "mydir/mysubdir/"}, dir, cmpGitRepoOpt))
+			doc: "git scheme, no url-fragment",
-
+			url: "git://github.com/user/repo.git",
-	dir, err = parseRemoteURL("git@github.com:user/repo.git")
+			expected: gitRepo{
-	assert.NilError(t, err)
+				remote: "git://github.com/user/repo.git",
-	assert.Check(t, is.DeepEqual(gitRepo{"git@github.com:user/repo.git", "master", ""}, dir, cmpGitRepoOpt))
+				ref:    "master",
-
+			},
-	dir, err = parseRemoteURL("git@github.com:user/repo.git#mybranch:mydir/mysubdir/")
+		},
-	assert.NilError(t, err)
+		{
-	assert.Check(t, is.DeepEqual(gitRepo{"git@github.com:user/repo.git", "mybranch", "mydir/mysubdir/"}, dir, cmpGitRepoOpt))
+			doc: "git scheme, with url-fragment",
+			url: "git://github.com/user/repo.git#mybranch:mydir/mysubdir/",
+			expected: gitRepo{
+				remote: "git://github.com/user/repo.git",
+				ref:    "mybranch",
+				subdir: "mydir/mysubdir/",
+			},
+		},
+		{
+			doc: "https scheme, no url-fragment",
+			url: "https://github.com/user/repo.git",
+			expected: gitRepo{
+				remote: "https://github.com/user/repo.git",
+				ref:    "master",
+			},
+		},
+		{
+			doc: "https scheme, with url-fragment",
+			url: "https://github.com/user/repo.git#mybranch:mydir/mysubdir/",
+			expected: gitRepo{
+				remote: "https://github.com/user/repo.git",
+				ref:    "mybranch",
+				subdir: "mydir/mysubdir/",
+			},
+		},
+		{
+			doc: "git@, no url-fragment",
+			url: "git@github.com:user/repo.git",
+			expected: gitRepo{
+				remote: "git@github.com:user/repo.git",
+				ref:    "master",
+			},
+		},
+		{
+			doc: "git@, with url-fragment",
+			url: "git@github.com:user/repo.git#mybranch:mydir/mysubdir/",
+			expected: gitRepo{
+				remote: "git@github.com:user/repo.git",
+				ref:    "mybranch",
+				subdir: "mydir/mysubdir/",
+			},
+		},
 	}
 
-var cmpGitRepoOpt = cmp.AllowUnexported(gitRepo{})
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.doc, func(t *testing.T) {
+			repo, err := parseRemoteURL(tc.url)
+			assert.NilError(t, err)
+			assert.Check(t, is.DeepEqual(tc.expected, repo, cmp.AllowUnexported(gitRepo{})))
+		})
+	}
+}
 
 func TestCloneArgsSmartHttp(t *testing.T) {
 	mux := http.NewServeMux()