Fix handling of remote "git@" notation

`docker build` accepts remote repositories
using either the `git://` notation, or `git@`.

Docker attempted to parse both as an URL, however,
`git@` is not an URL, but an argument to `git clone`.

Go 1.7 silently ignored this, and managed to
extract the needed information from these
remotes, however, Go 1.8 does a more strict
validation, and invalidated these.

This patch adds a different path for `git@` remotes,
to prevent them from being handled as URL (and
invalidated).

A test is also added, because there were no
tests for handling of `git@` remotes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

builder/remotecontext/git/gitutils.go

@@ -15,18 +15,24 @@ import (
 	"github.com/pkg/errors"
 )
 
+type gitRepo struct {
+	remote string
+	ref    string
+	subdir string
+}
+
 // Clone clones a repository into a newly created directory which
 // will be under "docker-build-git"
 func Clone(remoteURL string) (string, error) {
-	if !urlutil.IsGitTransport(remoteURL) {
-		remoteURL = "https://" + remoteURL
-	}
-	root, err := ioutil.TempDir("", "docker-build-git")
+	repo, err := parseRemoteURL(remoteURL)
+
 	if err != nil {
 		return "", err
 	}
 
-	u, err := url.Parse(remoteURL)
+	fetch := fetchArgs(repo.remote, repo.ref)
+
+	root, err := ioutil.TempDir("", "docker-build-git")
 	if err != nil {
 		return "", err
 	}
@@ -35,22 +41,47 @@ func Clone(remoteURL string) (string, error) {
 		return "", errors.Wrapf(err, "failed to init repo at %s: %s", root, out)
 	}
 
-	ref, subdir := getRefAndSubdir(u.Fragment)
-	fetch := fetchArgs(u, ref)
-
-	u.Fragment = ""
-
 	// Add origin remote for compatibility with previous implementation that
 	// used "git clone" and also to make sure local refs are created for branches
-	if out, err := gitWithinDir(root, "remote", "add", "origin", u.String()); err != nil {
-		return "", errors.Wrapf(err, "failed add origin repo at %s: %s", u.String(), out)
+	if out, err := gitWithinDir(root, "remote", "add", "origin", repo.remote); err != nil {
+		return "", errors.Wrapf(err, "failed add origin repo at %s: %s", repo.remote, out)
 	}
 
 	if output, err := gitWithinDir(root, fetch...); err != nil {
 		return "", errors.Wrapf(err, "error fetching: %s", output)
 	}
 
-	return checkoutGit(root, ref, subdir)
+	return checkoutGit(root, repo.ref, repo.subdir)
+}
+
+func parseRemoteURL(remoteURL string) (gitRepo, error) {
+	repo := gitRepo{}
+
+	if !urlutil.IsGitTransport(remoteURL) {
+		remoteURL = "https://" + remoteURL
+	}
+
+	var fragment string
+	if strings.HasPrefix(remoteURL, "git@") {
+		// git@.. is not an URL, so cannot be parsed as URL
+		parts := strings.SplitN(remoteURL, "#", 2)
+
+		repo.remote = parts[0]
+		if len(parts) == 2 {
+			fragment = parts[1]
+		}
+		repo.ref, repo.subdir = getRefAndSubdir(fragment)
+	} else {
+		u, err := url.Parse(remoteURL)
+		if err != nil {
+			return repo, err
+		}
+
+		repo.ref, repo.subdir = getRefAndSubdir(u.Fragment)
+		u.Fragment = ""
+		repo.remote = u.String()
+	}
+	return repo, nil
 }
 
 func getRefAndSubdir(fragment string) (ref string, subdir string) {
@@ -65,11 +96,11 @@ func getRefAndSubdir(fragment string) (ref string, subdir string) {
 	return
 }
 
-func fetchArgs(remoteURL *url.URL, ref string) []string {
+func fetchArgs(remoteURL string, ref string) []string {
 	args := []string{"fetch", "--recurse-submodules=yes"}
 	shallow := true
 
-	if strings.HasPrefix(remoteURL.Scheme, "http") {
+	if urlutil.IsURL(remoteURL) {
 		res, err := http.Head(fmt.Sprintf("%s/info/refs?service=git-upload-pack", remoteURL))
 		if err != nil || res.Header.Get("Content-Type") != "application/x-git-upload-pack-advertisement" {
 			shallow = false

builder/remotecontext/git/gitutils_test.go

@@ -16,6 +16,38 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestParseRemoteURL(t *testing.T) {
+	dir, err := parseRemoteURL("git://github.com/user/repo.git")
+	require.NoError(t, err)
+	assert.NotEmpty(t, dir)
+	assert.Equal(t, gitRepo{"git://github.com/user/repo.git", "master", ""}, dir)
+
+	dir, err = parseRemoteURL("git://github.com/user/repo.git#mybranch:mydir/mysubdir/")
+	require.NoError(t, err)
+	assert.NotEmpty(t, dir)
+	assert.Equal(t, gitRepo{"git://github.com/user/repo.git", "mybranch", "mydir/mysubdir/"}, dir)
+
+	dir, err = parseRemoteURL("https://github.com/user/repo.git")
+	require.NoError(t, err)
+	assert.NotEmpty(t, dir)
+	assert.Equal(t, gitRepo{"https://github.com/user/repo.git", "master", ""}, dir)
+
+	dir, err = parseRemoteURL("https://github.com/user/repo.git#mybranch:mydir/mysubdir/")
+	require.NoError(t, err)
+	assert.NotEmpty(t, dir)
+	assert.Equal(t, gitRepo{"https://github.com/user/repo.git", "mybranch", "mydir/mysubdir/"}, dir)
+
+	dir, err = parseRemoteURL("git@github.com:user/repo.git")
+	require.NoError(t, err)
+	assert.NotEmpty(t, dir)
+	assert.Equal(t, gitRepo{"git@github.com:user/repo.git", "master", ""}, dir)
+
+	dir, err = parseRemoteURL("git@github.com:user/repo.git#mybranch:mydir/mysubdir/")
+	require.NoError(t, err)
+	assert.NotEmpty(t, dir)
+	assert.Equal(t, gitRepo{"git@github.com:user/repo.git", "mybranch", "mydir/mysubdir/"}, dir)
+}
+
 func TestCloneArgsSmartHttp(t *testing.T) {
 	mux := http.NewServeMux()
 	server := httptest.NewServer(mux)
@@ -28,7 +60,7 @@ func TestCloneArgsSmartHttp(t *testing.T) {
 		w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-advertisement", q))
 	})
 
-	args := fetchArgs(serverURL, "master")
+	args := fetchArgs(serverURL.String(), "master")
 	exp := []string{"fetch", "--recurse-submodules=yes", "--depth", "1", "origin", "master"}
 	assert.Equal(t, exp, args)
 }
@@ -44,14 +76,13 @@ func TestCloneArgsDumbHttp(t *testing.T) {
 		w.Header().Set("Content-Type", "text/plain")
 	})
 
-	args := fetchArgs(serverURL, "master")
+	args := fetchArgs(serverURL.String(), "master")
 	exp := []string{"fetch", "--recurse-submodules=yes", "origin", "master"}
 	assert.Equal(t, exp, args)
 }
 
 func TestCloneArgsGit(t *testing.T) {
-	u, _ := url.Parse("git://github.com/docker/docker")
-	args := fetchArgs(u, "master")
+	args := fetchArgs("git://github.com/docker/docker", "master")
 	exp := []string{"fetch", "--recurse-submodules=yes", "--depth", "1", "origin", "master"}
 	assert.Equal(t, exp, args)
 }