feat: use git[+subprotocol] pattern

See helm/community#321 Signed-off-by: Dominykas Blyžė <[email protected]>
dominykas · Dec 15, 2023 · 5843b82 · 5843b82
1 parent 4dc162f
commit 5843b82
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 16 deletions.
diff --git a/internal/gitutil/gitutil.go b/internal/gitutil/gitutil.go
@@ -18,11 +18,14 @@ package gitutil
 
 import (
 	"os"
+	"regexp"
 	"strings"
 
 	"github.com/Masterminds/vcs"
 )
 
+var gitRepositoryURLRe = regexp.MustCompile(`^git(\+\w+)?://`)
+
 // HasGitReference returns true if a git repository contains a specified ref (branch/tag)
 func HasGitReference(gitRepo, ref, repoName string) (bool, error) {
 	local, err := os.MkdirTemp("", repoName)
@@ -44,10 +47,10 @@ func HasGitReference(gitRepo, ref, repoName string) (bool, error) {
 
 // IsGitRepository determines whether a URL is to be treated as a git repository URL
 func IsGitRepository(url string) bool {
-	return strings.HasPrefix(url, "git://")
+	return gitRepositoryURLRe.MatchString(url)
 }
 
 // RepositoryURLToGitURL converts a repository URL into a URL that `git clone` could consume
 func RepositoryURLToGitURL(url string) string {
-	return strings.TrimPrefix(url, "git://")
+	return strings.TrimPrefix(url, "git+")
 }
diff --git a/internal/gitutil/gitutil_test.go b/internal/gitutil/gitutil_test.go
@@ -28,7 +28,7 @@ func TestIsGitUrl(t *testing.T) {
 	}{
 		{"oci://example.com/example/chart", false},
 		{"git://example.com/example/chart", true},
-		{"git://https://example.com/example/chart", true},
+		{"git+https://example.com/example/chart", true},
 	}
 
 	for _, test := range tests {
@@ -37,3 +37,20 @@ func TestIsGitUrl(t *testing.T) {
 		}
 	}
 }
+
+func TestRepositoryURLToGitURL(t *testing.T) {
+	// Test table: Given url, RepositoryURLToGitURL should return expect.
+	tests := []struct {
+		url    string
+		expect string
+	}{
+		{"git://example.com/example/chart", "git://example.com/example/chart"},
+		{"git+https://example.com/example/chart", "https://example.com/example/chart"},
+	}
+
+	for _, test := range tests {
+		if RepositoryURLToGitURL(test.url) != test.expect {
+			t.Errorf("Expected %s for %s", test.expect, test.url)
+		}
+	}
+}
diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go
@@ -149,47 +149,47 @@ func TestResolve(t *testing.T) {
 		{
 			name: "repo from git https url",
 			req: []*chart.Dependency{
-				{Name: "gitdependencyok", Repository: "git://https://github.com/helm/helmchart.git", Version: "1.0.0"},
+				{Name: "gitdependencyok", Repository: "git+https://github.com/helm/helmchart.git", Version: "1.0.0"},
 			},
 			expect: &chart.Lock{
 				Dependencies: []*chart.Dependency{
-					{Name: "gitdependencyok", Repository: "git://https://github.com/helm/helmchart.git", Version: "1.0.0"},
+					{Name: "gitdependencyok", Repository: "git+https://github.com/helm/helmchart.git", Version: "1.0.0"},
 				},
 			},
 			err: false,
 		},
 		{
 			name: "repo from git https url",
 			req: []*chart.Dependency{
-				{Name: "gitdependencyerror", Repository: "git://https://github.com/helm/helmchart.git", Version: "2.0.0"},
+				{Name: "gitdependencyerror", Repository: "git+https://github.com/helm/helmchart.git", Version: "2.0.0"},
 			},
 			expect: &chart.Lock{
 				Dependencies: []*chart.Dependency{
-					{Name: "gitdependencyerror", Repository: "git://https://github.com/helm/helmchart.git", Version: "2.0.0"},
+					{Name: "gitdependencyerror", Repository: "git+https://github.com/helm/helmchart.git", Version: "2.0.0"},
 				},
 			},
 			err: true,
 		},
 		{
 			name: "repo from git ssh url",
 			req: []*chart.Dependency{
-				{Name: "gitdependency", Repository: "git://git@github.com:helm/helmchart.git", Version: "1.0.0"},
+				{Name: "gitdependency", Repository: "git://github.com:helm/helmchart.git", Version: "1.0.0"},
 			},
 			expect: &chart.Lock{
 				Dependencies: []*chart.Dependency{
-					{Name: "gitdependency", Repository: "git://git@github.com:helm/helmchart.git", Version: "1.0.0"},
+					{Name: "gitdependency", Repository: "git://github.com:helm/helmchart.git", Version: "1.0.0"},
 				},
 			},
 			err: false,
 		},
 		{
 			name: "repo from git ssh url",
 			req: []*chart.Dependency{
-				{Name: "gitdependencyerror", Repository: "git://git@github.com:helm/helmchart.git", Version: "2.0.0"},
+				{Name: "gitdependencyerror", Repository: "git://github.com:helm/helmchart.git", Version: "2.0.0"},
 			},
 			expect: &chart.Lock{
 				Dependencies: []*chart.Dependency{
-					{Name: "gitdependencyerror", Repository: "git://git@github.com:helm/helmchart.git", Version: "2.0.0"},
+					{Name: "gitdependencyerror", Repository: "git://github.com:helm/helmchart.git", Version: "2.0.0"},
 				},
 			},
 			err: true,

diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go
@@ -40,7 +40,8 @@ func TestResolveChartRef(t *testing.T) {
 		{name: "full URL", ref: "http://example.com/foo-1.2.3.tgz", expect: "http://example.com/foo-1.2.3.tgz"},
 		{name: "full URL, HTTPS", ref: "https://example.com/foo-1.2.3.tgz", expect: "https://example.com/foo-1.2.3.tgz"},
 		{name: "full URL, with authentication", ref: "http://username:[email protected]/foo-1.2.3.tgz", expect: "http://username:[email protected]/foo-1.2.3.tgz"},
-		{name: "helmchart", ref: "git://https://github.com/helmchart/helmchart.git", expect: "https://github.com/helmchart/helmchart.git"},
+		{name: "helmchart", ref: "git+https://github.com/helmchart/helmchart.git", expect: "https://github.com/helmchart/helmchart.git"},
+		{name: "helmchart", ref: "git://github.com/helmchart/helmchart.git", expect: "git://github.com/helmchart/helmchart.git"},
 		{name: "reference, testing repo", ref: "testing/alpine", expect: "http://example.com/alpine-1.2.3.tgz"},
 		{name: "reference, version, testing repo", ref: "testing/alpine", version: "0.2.0", expect: "http://example.com/alpine-0.2.0.tgz"},
 		{name: "reference, version, malformed repo", ref: "malformed/alpine", version: "1.2.3", expect: "http://dl.example.com/alpine-1.2.3.tgz"},

diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go
@@ -86,7 +86,7 @@ func TestFindChartURL(t *testing.T) {
 	}{
 		{name: "alpine", version: "0.1.0", repoURL: "http://example.com/charts", expectChurl: "https://charts.helm.sh/stable/alpine-0.1.0.tgz", expectUserName: "", expectPassword: "", expectInsecureSkipTLSVerify: false, expectPasscredentialsall: false},
 		{name: "tlsfoo", version: "1.2.3", repoURL: "https://example-https-insecureskiptlsverify.com", expectChurl: "https://example.com/tlsfoo-1.2.3.tgz", expectUserName: "", expectPassword: "", expectInsecureSkipTLSVerify: true, expectPasscredentialsall: false},
-		{name: "helm-test", version: "master", repoURL: "git://https://github.com/rally25rs/helm-test-chart.git", expectChurl: "git://https://github.com/rally25rs/helm-test-chart.git", expectUserName: "", expectPassword: "", expectInsecureSkipTLSVerify: false, expectPasscredentialsall: false},
+		{name: "helm-test", version: "master", repoURL: "git+https://github.com/rally25rs/helm-test-chart.git", expectChurl: "git+https://github.com/rally25rs/helm-test-chart.git", expectUserName: "", expectPassword: "", expectInsecureSkipTLSVerify: false, expectPasscredentialsall: false},
 	}
 	for _, tt := range tests {
 		churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(tt.name, tt.version, tt.repoURL, repos)
@@ -177,9 +177,9 @@ func TestGetRepoNames(t *testing.T) {
 		{
 			name: "repo from git url",
 			req: []*chart.Dependency{
-				{Name: "local-dep", Repository: "git://https://github.com/git/git"},
+				{Name: "local-dep", Repository: "git+https://github.com/git/git"},
 			},
-			expect: map[string]string{"local-dep": "git://https://github.com/git/git"},
+			expect: map[string]string{"local-dep": "git+https://github.com/git/git"},
 		},
 	}
 

diff --git a/pkg/getter/gitgetter.go b/pkg/getter/gitgetter.go
@@ -18,10 +18,11 @@ package getter
 import (
 	"bytes"
 	"fmt"
-	"helm.sh/helm/v3/pkg/gitutil"
 	"os"
 	"path/filepath"
 
+	"helm.sh/helm/v3/pkg/gitutil"
+
 	"github.com/Masterminds/vcs"
 
 	"helm.sh/helm/v3/internal/fileutil"