From 0bfbfc30f754a08d013a18359386f6ef7aa481de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Fri, 15 Dec 2023 18:17:18 +0200 Subject: [PATCH] feat: use git[+subprotocol] pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/helm/community/pull/321 Signed-off-by: Dominykas Blyžė --- internal/resolver/resolver_test.go | 16 ++++++++-------- pkg/downloader/chart_downloader_test.go | 3 ++- pkg/downloader/manager_test.go | 6 +++--- pkg/getter/gitgetter.go | 3 ++- pkg/gitutil/gitutil.go | 7 +++++-- pkg/gitutil/gitutil_test.go | 19 ++++++++++++++++++- 6 files changed, 38 insertions(+), 16 deletions(-) diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 54fbf939fd8..5c970e1b74a 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -149,11 +149,11 @@ 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, @@ -161,11 +161,11 @@ func TestResolve(t *testing.T) { { 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, @@ -173,11 +173,11 @@ func TestResolve(t *testing.T) { { 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, @@ -185,11 +185,11 @@ func TestResolve(t *testing.T) { { 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 index 54302c9967f..fd5a64c7adb 100644 --- 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:password@example.com/foo-1.2.3.tgz", expect: "http://username:password@example.com/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 index 0652f644057..48fcc3a2d10 100644 --- 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 index 4fa80df63df..7f3546b16af 100644 --- 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" diff --git a/pkg/gitutil/gitutil.go b/pkg/gitutil/gitutil.go index 0c8784b8873..40d5c324fc2 100644 --- a/pkg/gitutil/gitutil.go +++ b/pkg/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/pkg/gitutil/gitutil_test.go b/pkg/gitutil/gitutil_test.go index aa8b635f8f4..7a8c08a6551 100644 --- a/pkg/gitutil/gitutil_test.go +++ b/pkg/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) + } + } +}