Repository: go-kratos/kratos
Pull request #2295 (2022-08-19 02:54:53)

fix `kratos new -r` not support scp styled git url.issue https://gith…
ref https://github.com/golang/go/blob/master/src/cmd/go/internal/vcs/vcs.go
see https://go-review.googlesource.com/c/go/+/12226/
git url define https://git-scm.com/docs/git-clone#_git_urls

codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/2295?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#2295](https://codecov.io/gh/go-kratos/kratos/pull/2295?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (5dbb3aa) into [main](https://codecov.io/gh/go-kratos/kratos/commit/6ca225b078b7666de95895b2d645887b0c07e9be?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (6ca225b) will **increase** coverage by `0.05%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##             main    #2295      +/-   ##
==========================================
+ Coverage   84.91%   84.96%   +0.05%     
==========================================
  Files          88       88              
  Lines        3771     3771              
==========================================
+ Hits         3202     3204       +2     
+ Misses        401      399       -2     
  Partials      168      168              
```


| [Impacted Files](https://codecov.io/gh/go-kratos/kratos/pull/2295?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [internal/context/context.go](https://codecov.io/gh/go-kratos/kratos/pull/2295/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-aW50ZXJuYWwvY29udGV4dC9jb250ZXh0Lmdv) | `100.00% <0.00%> (+2.98%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)

shenqidebaozi: please fix lint
czyt: @shenqidebaozi lint fixed
czyt: @Windfarer hello,code issues fixed,pls review 😉

Patch:
diff --git a/README.md b/README.md
index f4023812..815be2fc 100644
--- a/README.md
+++ b/README.md
@@ -90,7 +90,6 @@ If you discover a security vulnerability within Kratos, please send an e-mail to
 - [Wechat Group](https://github.com/go-kratos/kratos/issues/682)
 - [Discord Group](https://discord.gg/BWzJsUJ)
 - [go-kratos.dev](https://go-kratos.dev/en)
-- QQ Group: 716486124
 
 ## Contributors
 
diff --git a/cmd/kratos/internal/base/repo.go b/cmd/kratos/internal/base/repo.go
index 55b12ac0..bdcb3f01 100644
--- a/cmd/kratos/internal/base/repo.go
+++ b/cmd/kratos/internal/base/repo.go
@@ -3,13 +3,15 @@ package base
 import (
 	"context"
 	"fmt"
-	stdurl "net/url"
+	"net"
 	"os"
 	"os/exec"
 	"path"
 	"strings"
 )
 
+var unExpandVarPath = []string{"~", ".", ".."}
+
 // Repo is git repository manager.
 type Repo struct {
 	url    string
@@ -18,27 +20,21 @@ type Repo struct {
 }
 
 func repoDir(url string) string {
-	if !strings.Contains(url, "//") {
-		url = "//" + url
-	}
-	if strings.HasPrefix(url, "//git@") {
-		url = "ssh:" + url
-	} else if strings.HasPrefix(url, "//") {
-		url = "https:" + url
+	vcsURL, err := ParseVCSUrl(url)
+	if err != nil {
+		return url
 	}
-	u, err := stdurl.Parse(url)
-	if err == nil {
-		url = fmt.Sprintf("%s://%s%s", u.Scheme, u.Hostname(), u.Path)
+	// check host contains port
+	host, _, err := net.SplitHostPort(vcsURL.Host)
+	if err != nil {
+		host = vcsURL.Host
 	}
-	var start int
-	start = strings.Index(url, "//")
-	if start == -1 {
-		start = strings.Index(url, ":") + 1
-	} else {
-		start += 2
+	for _, p := range unExpandVarPath {
+		host = strings.TrimLeft(host, p)
 	}
-	end := strings.LastIndex(url, "/")
-	return url[start:end]
+	dir := path.Base(path.Dir(vcsURL.Path))
+	url = fmt.Sprintf("%s/%s", host, dir)
+	return url
 }
 
 // NewRepo new a repository manager.
diff --git a/cmd/kratos/internal/base/vcs_url.go b/cmd/kratos/internal/base/vcs_url.go
new file mode 100644
index 00000000..ef76db30
--- /dev/null
+++ b/cmd/kratos/internal/base/vcs_url.go
@@ -0,0 +1,58 @@
+package base
+
+import (
+	"errors"
+	"net/url"
+	"regexp"
+	"strings"
+)
+
+var (
+	scpSyntaxRe = regexp.MustCompile(`^([a-zA-Z0-9_]+)@([a-zA-Z0-9._-]+):(.*)$`)
+	scheme      = []string{"git", "https", "http", "git+ssh", "ssh", "file", "ftp", "ftps"}
+)
+
+// ParseVCSUrl ref https://github.com/golang/go/blob/master/src/cmd/go/internal/vcs/vcs.go
+// see https://go-review.googlesource.com/c/go/+/12226/
+// git url define https://git-scm.com/docs/git-clone#_git_urls
+func ParseVCSUrl(repo string) (*url.URL, error) {
+	var (
+		repoURL *url.URL
+		err     error
+	)
+
+	if m := scpSyntaxRe.FindStringSubmatch(repo); m != nil {
+		// Match SCP-like syntax and convert it to a URL.
+		// Eg, "git@github.com:user/repo" becomes
+		// "ssh://git@github.com/user/repo".
+		repoURL = &url.URL{
+			Scheme: "ssh",
+			User:   url.User(m[1]),
+			Host:   m[2],
+			Path:   m[3],
+		}
+	} else {
+		if !strings.Contains(repo, "//") {
+			repo = "//" + repo
+		}
+		if strings.HasPrefix(repo, "//git@") {
+			repo = "ssh:" + repo
+		} else if strings.HasPrefix(repo, "//") {
+			repo = "https:" + repo
+		}
+		repoURL, err = url.Parse(repo)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// Iterate over insecure schemes too, because this function simply
+	// reports the state of the repo. If we can't see insecure schemes then
+	// we can't report the actual repo URL.
+	for _, s := range scheme {
+		if repoURL.Scheme == s {
+			return repoURL, nil
+		}
+	}
+	return nil, errors.New("unable to parse repo url")
+}


Test patch:
diff --git a/cmd/kratos/internal/base/repo_test.go b/cmd/kratos/internal/base/repo_test.go
index 15ed33c8..dfcdbb93 100644
--- a/cmd/kratos/internal/base/repo_test.go
+++ b/cmd/kratos/internal/base/repo_test.go
@@ -7,7 +7,37 @@ import (
 )
 
 func TestRepo(t *testing.T) {
-	os.RemoveAll("/tmp/test_repo")
+	urls := []string{
+		// ssh://[user@]host.xz[:port]/path/to/repo.git/
+		"ssh://git@github.com:7875/go-kratos/kratos.git",
+		// git://host.xz[:port]/path/to/repo.git/
+		"git://github.com:7875/go-kratos/kratos.git",
+		// http[s]://host.xz[:port]/path/to/repo.git/
+		"https://github.com:7875/go-kratos/kratos.git",
+		// ftp[s]://host.xz[:port]/path/to/repo.git/
+		"ftps://github.com:7875/go-kratos/kratos.git",
+		//[user@]host.xz:path/to/repo.git/
+		"git@github.com:go-kratos/kratos.git",
+		// ssh://[user@]host.xz[:port]/~[user]/path/to/repo.git/
+		"ssh://git@github.com:7875/go-kratos/kratos.git",
+		// git://host.xz[:port]/~[user]/path/to/repo.git/
+		"git://github.com:7875/go-kratos/kratos.git",
+		//[user@]host.xz:/~[user]/path/to/repo.git/
+		"git@github.com:go-kratos/kratos.git",
+		///path/to/repo.git/
+		"//github.com/go-kratos/kratos.git",
+		// file:///path/to/repo.git/
+		"file://./github.com/go-kratos/kratos.git",
+	}
+	for _, url := range urls {
+		dir := repoDir(url)
+		if dir != "github.com/go-kratos" && dir != "/go-kratos" {
+			t.Fatal(url, "repoDir test failed", dir)
+		}
+	}
+}
+
+func TestRepoClone(t *testing.T) {
 	r := NewRepo("https://github.com/go-kratos/service-layout.git", "")
 	if err := r.Clone(context.Background()); err != nil {
 		t.Fatal(err)
@@ -15,19 +45,7 @@ func TestRepo(t *testing.T) {
 	if err := r.CopyTo(context.Background(), "/tmp/test_repo", "github.com/go-kratos/kratos-layout", nil); err != nil {
 		t.Fatal(err)
 	}
-	urls := []string{
-		"ssh://git@gitlab.xxx.com:1234/foo/bar.git",
-		"ssh://gitlab.xxx.com:1234/foo/bar.git",
-		"//git@gitlab.xxx.com:1234/foo/bar.git",
-		"git@gitlab.xxx.com:1234/foo/bar.git",
-		"gitlab.xxx.com:1234/foo/bar.git",
-		"gitlab.xxx.com/foo/bar.git",
-		"gitlab.xxx.com/foo/bar",
-	}
-	for _, url := range urls {
-		dir := repoDir(url)
-		if dir != "gitlab.xxx.com/foo" {
-			t.Fatal("repoDir test failed", dir)
-		}
-	}
+	t.Cleanup(func() {
+		os.RemoveAll("/tmp/test_repo")
+	})
 }
diff --git a/cmd/kratos/internal/base/vcs_url_test.go b/cmd/kratos/internal/base/vcs_url_test.go
new file mode 100644
index 00000000..09b8d568
--- /dev/null
+++ b/cmd/kratos/internal/base/vcs_url_test.go
@@ -0,0 +1,55 @@
+package base
+
+import (
+	"net"
+	"strings"
+	"testing"
+)
+
+func TestParseVCSUrl(t *testing.T) {
+	repos := []string{
+		// ssh://[user@]host.xz[:port]/path/to/repo.git/
+		"ssh://git@github.com:7875/go-kratos/kratos.git",
+		// git://host.xz[:port]/path/to/repo.git/
+		"git://github.com:7875/go-kratos/kratos.git",
+		// http[s]://host.xz[:port]/path/to/repo.git/
+		"https://github.com:7875/go-kratos/kratos.git",
+		// ftp[s]://host.xz[:port]/path/to/repo.git/
+		"ftps://github.com:7875/go-kratos/kratos.git",
+		//[user@]host.xz:path/to/repo.git/
+		"git@github.com:go-kratos/kratos.git",
+		// ssh://[user@]host.xz[:port]/~[user]/path/to/repo.git/
+		"ssh://git@github.com:7875/go-kratos/kratos.git",
+		// git://host.xz[:port]/~[user]/path/to/repo.git/
+		"git://github.com:7875/go-kratos/kratos.git",
+		//[user@]host.xz:/~[user]/path/to/repo.git/
+		"git@github.com:go-kratos/kratos.git",
+		///path/to/repo.git/
+		"~/go-kratos/kratos.git",
+		// file:///path/to/repo.git/
+		"file://~/go-kratos/kratos.git",
+	}
+	for _, repo := range repos {
+		url, err := ParseVCSUrl(repo)
+		if err != nil {
+			t.Fatal(repo, err)
+		}
+		urlPath := strings.TrimLeft(url.Path, "/")
+		if urlPath != "go-kratos/kratos.git" {
+			t.Fatal(repo, "parse url failed", urlPath)
+		}
+	}
+}
+
+func TestParseSsh(t *testing.T) {
+	repo := "ssh://git@github.com:7875/go-kratos/kratos.git"
+	url, err := ParseVCSUrl(repo)
+	if err != nil {
+		t.Fatal(err)
+	}
+	host, _, err := net.SplitHostPort(url.Host)
+	if err != nil {
+		host = url.Host
+	}
+	t.Log(host, url.Path)
+}
diff --git a/selector/selector_test.go b/selector/selector_test.go
index f7c1f1bc..bcc08264 100644
--- a/selector/selector_test.go
+++ b/selector/selector_test.go
@@ -12,6 +12,8 @@ import (
 	"github.com/go-kratos/kratos/v2/registry"
 )
 
+var errNodeNotMatch = errors.New("node is not match")
+
 type mockWeightedNode struct {
 	Node
 
@@ -80,6 +82,18 @@ func (b *mockBalancer) Pick(ctx context.Context, nodes []WeightedNode) (selected
 	return
 }
 
+type mockMustErrorBalancerBuilder struct{}
+
+func (b *mockMustErrorBalancerBuilder) Build() Balancer {
+	return &mockMustErrorBalancer{}
+}
+
+type mockMustErrorBalancer struct{}
+
+func (b *mockMustErrorBalancer) Pick(ctx context.Context, nodes []WeightedNode) (selected WeightedNode, done DoneFunc, err error) {
+	return nil, nil, errNodeNotMatch
+}
+
 func TestDefault(t *testing.T) {
 	builder := DefaultBuilder{
 		Node:     &mockWeightedNodeBuilder{},
@@ -107,6 +121,7 @@ func TestDefault(t *testing.T) {
 			Endpoints: []string{"http://127.0.0.1:9090"},
 			Metadata:  map[string]string{"weight": "10"},
 		}))
+
 	selector.Apply(nodes)
 	n, done, err := selector.Select(context.Background(), WithNodeFilter(mockFilter("v2.0.0")))
 	if err != nil {
@@ -121,6 +136,9 @@ func TestDefault(t *testing.T) {
 	if !reflect.DeepEqual("v2.0.0", n.Version()) {
 		t.Errorf("expect %v, got %v", "v2.0.0", n.Version())
 	}
+	if n.Scheme() == "" {
+		t.Errorf("expect %v, got %v", "", n.Scheme())
+	}
 	if n.Address() == "" {
 		t.Errorf("expect %v, got %v", "", n.Address())
 	}
@@ -135,6 +153,21 @@ func TestDefault(t *testing.T) {
 	}
 	done(context.Background(), DoneInfo{})
 
+	// peer in ctx
+	ctx := NewPeerContext(context.Background(), &Peer{
+		Node: mockWeightedNode{},
+	})
+	n, done, err = selector.Select(ctx)
+	if err != nil {
+		t.Errorf("expect %v, got %v", ErrNoAvailable, err)
+	}
+	if done == nil {
+		t.Errorf("expect %v, got %v", nil, done)
+	}
+	if n == nil {
+		t.Errorf("expect %v, got %v", nil, n)
+	}
+
 	// no v3.0.0 instance
 	n, done, err = selector.Select(context.Background(), WithNodeFilter(mockFilter("v3.0.0")))
 	if !errors.Is(ErrNoAvailable, err) {
@@ -172,4 +205,87 @@ func TestDefault(t *testing.T) {
 	if n != nil {
 		t.Errorf("expect %v, got %v", nil, n)
 	}
+
+	// without node_filters
+	n, done, err = selector.Select(context.Background())
+	if !errors.Is(ErrNoAvailable, err) {
+		t.Errorf("expect %v, got %v", ErrNoAvailable, err)
+	}
+	if done != nil {
+		t.Errorf("expect %v, got %v", nil, done)
+	}
+	if n != nil {
+		t.Errorf("expect %v, got %v", nil, n)
+	}
+}
+
+func TestWithoutApply(t *testing.T) {
+	builder := DefaultBuilder{
+		Node:     &mockWeightedNodeBuilder{},
+		Balancer: &mockBalancerBuilder{},
+	}
+	selector := builder.Build()
+	n, done, err := selector.Select(context.Background())
+	if !errors.Is(ErrNoAvailable, err) {
+		t.Errorf("expect %v, got %v", ErrNoAvailable, err)
+	}
+	if done != nil {
+		t.Errorf("expect %v, got %v", nil, done)
+	}
+	if n != nil {
+		t.Errorf("expect %v, got %v", nil, n)
+	}
+}
+
+func TestNoPick(t *testing.T) {
+	builder := DefaultBuilder{
+		Node:     &mockWeightedNodeBuilder{},
+		Balancer: &mockMustErrorBalancerBuilder{},
+	}
+	var nodes []Node
+	nodes = append(nodes, NewNode(
+		"http",
+		"127.0.0.1:8080",
+		&registry.ServiceInstance{
+			ID:        "127.0.0.1:8080",
+			Name:      "helloworld",
+			Version:   "v2.0.0",
+			Endpoints: []string{"http://127.0.0.1:8080"},
+			Metadata:  map[string]string{"weight": "10"},
+		}))
+	nodes = append(nodes, NewNode(
+		"http",
+		"127.0.0.1:9090",
+		&registry.ServiceInstance{
+			ID:        "127.0.0.1:9090",
+			Name:      "helloworld",
+			Version:   "v1.0.0",
+			Endpoints: []string{"http://127.0.0.1:9090"},
+			Metadata:  map[string]string{"weight": "10"},
+		}))
+	selector := builder.Build()
+	selector.Apply(nodes)
+	n, done, err := selector.Select(context.Background())
+	if !errors.Is(errNodeNotMatch, err) {
+		t.Errorf("expect %v, got %v", errNodeNotMatch, err)
+	}
+	if done != nil {
+		t.Errorf("expect %v, got %v", nil, done)
+	}
+	if n != nil {
+		t.Errorf("expect %v, got %v", nil, n)
+	}
+}
+
+func TestGolobal(t *testing.T) {
+	builder := DefaultBuilder{
+		Node:     &mockWeightedNodeBuilder{},
+		Balancer: &mockBalancerBuilder{},
+	}
+	SetGlobalSelector(&builder)
+
+	gBuilder := GlobalSelector()
+	if gBuilder == nil {
+		t.Errorf("expect %v, got %v", nil, gBuilder)
+	}
 }