feat: support charts from sub-folders inside git repository

Signed-off-by: Dominykas Blyžė <hello@dominykas.com>
dominykas · Feb 6, 2024 · 3ad7c79 · 3ad7c79
1 parent 9b037f2
commit 3ad7c79
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 21 deletions.
diff --git a/internal/gitutil/gitutil.go b/internal/gitutil/gitutil.go
@@ -31,8 +31,9 @@ import (
 var gitRepositoryURLRe = regexp.MustCompile(`^git(\+\w+)?://`)
 
 type GitRepositoryURL struct {
-	RepositoryURL string
-	GitRemoteURL  *url.URL
+	RepositoryURL          string
+	GitRemoteURL           *url.URL
+	PathUnderGitRepository string
 }
 
 // HasGitReference returns true if a git repository contains a specified ref (branch/tag)
@@ -71,8 +72,22 @@ func ParseGitRepositoryURL(repositoryURL string) (*GitRepositoryURL, error) {
 		return nil, errors.Errorf("git repository URL should not contain credentials - please use git credential helpers")
 	}
 
+	path := ""
+
+	if gitRemoteURL.Fragment != "" {
+		query, err := url.ParseQuery(gitRemoteURL.Fragment)
+		if err != nil {
+			return nil, err
+		}
+
+		path = query.Get("subdirectory")
+	}
+
+	gitRemoteURL.Fragment = ""
+
 	return &GitRepositoryURL{
-		RepositoryURL: repositoryURL,
-		GitRemoteURL:  gitRemoteURL,
+		RepositoryURL:          repositoryURL,
+		GitRemoteURL:           gitRemoteURL,
+		PathUnderGitRepository: path,
 	}, err
 }
diff --git a/internal/gitutil/gitutil_test.go b/internal/gitutil/gitutil_test.go
@@ -41,29 +41,39 @@ func TestIsGitUrl(t *testing.T) {
 func TestParseGitRepositoryURL(t *testing.T) {
 	// Test table: Given url, ParseGitRepositoryURL should return expect.
 	tests := []struct {
-		url                    string
-		expectRepositoryURL    string
-		expectGitRepositoryURL string
+		url                            string
+		expectRepositoryURL            string
+		expectGitRemoteURL             string
+		expectedPathUnderGitRepository string
 	}{
 		{
-			url:                    "git://example.com/example/chart",
-			expectRepositoryURL:    "git://example.com/example/chart",
-			expectGitRepositoryURL: "git://example.com/example/chart",
+			url:                 "git://example.com/example/chart",
+			expectRepositoryURL: "git://example.com/example/chart",
+			expectGitRemoteURL:  "git://example.com/example/chart",
 		},
 		{
-			url:                    "git+https://example.com/example/chart",
-			expectRepositoryURL:    "git+https://example.com/example/chart",
-			expectGitRepositoryURL: "https://example.com/example/chart",
+			url:                 "git+https://example.com/example/chart",
+			expectRepositoryURL: "git+https://example.com/example/chart",
+			expectGitRemoteURL:  "https://example.com/example/chart",
+		},
+		{
+			url:                            "git+https://example.com/example/chart#subdirectory=charts/some-chart",
+			expectRepositoryURL:            "git+https://example.com/example/chart#subdirectory=charts/some-chart",
+			expectGitRemoteURL:             "https://example.com/example/chart",
+			expectedPathUnderGitRepository: "charts/some-chart",
 		},
 	}
 
 	for _, test := range tests {
 		parsed, _ := ParseGitRepositoryURL(test.url)
 		if parsed.RepositoryURL != test.expectRepositoryURL {
-			t.Errorf("Expected RepositoryURL %s for %s, but got %s", test.expectRepositoryURL, test.url, parsed)
+			t.Errorf("Expected RepositoryURL %s for %s, but got %s", test.expectRepositoryURL, test.url, parsed.RepositoryURL)
+		}
+		if parsed.GitRemoteURL.String() != test.expectGitRemoteURL {
+			t.Errorf("Expected GitRemoteURL %s for %s, but got %s", test.expectGitRemoteURL, test.url, parsed.GitRemoteURL)
 		}
-		if parsed.GitRemoteURL.String() != test.expectGitRepositoryURL {
-			t.Errorf("Expected GitRemoteURL %s for %s, but got %s", test.expectGitRepositoryURL, test.url, parsed)
+		if parsed.PathUnderGitRepository != test.expectedPathUnderGitRepository {
+			t.Errorf("Expected PathUnderGitRepository %s for %s, but got %s", test.expectGitRemoteURL, test.url, parsed.PathUnderGitRepository)
 		}
 	}
 }
diff --git a/pkg/getter/gitgetter.go b/pkg/getter/gitgetter.go
@@ -19,12 +19,12 @@ import (
 	"bytes"
 	"fmt"
 	"os"
-	"path/filepath"
 
 	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/chartutil"
 
 	"github.com/Masterminds/vcs"
+	securejoin "github.com/cyphar/filepath-securejoin"
 
 	"helm.sh/helm/v3/internal/gitutil"
 )
@@ -60,14 +60,18 @@ func (g *GitGetter) get(href string) (*bytes.Buffer, error) {
 	if err != nil {
 		return nil, err
 	}
-	chartTmpDir := filepath.Join(tmpDir, chartName)
 
-	if err := os.MkdirAll(chartTmpDir, 0755); err != nil {
+	gitTmpDir, err := securejoin.SecureJoin(tmpDir, chartName)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := os.MkdirAll(gitTmpDir, 0755); err != nil {
 		return nil, err
 	}
 	defer os.RemoveAll(tmpDir)
 
-	repo, err := vcs.NewRepo(gitURL.GitRemoteURL.String(), chartTmpDir)
+	repo, err := vcs.NewRepo(gitURL.GitRemoteURL.String(), gitTmpDir)
 	if err != nil {
 		return nil, err
 	}
@@ -78,7 +82,12 @@ func (g *GitGetter) get(href string) (*bytes.Buffer, error) {
 		return nil, err
 	}
 
-	ch, err := loader.LoadDir(chartTmpDir)
+	chartDir, err := securejoin.SecureJoin(gitTmpDir, gitURL.PathUnderGitRepository)
+	if err != nil {
+		return nil, err
+	}
+
+	ch, err := loader.LoadDir(chartDir)
 	if err != nil {
 		return nil, err
 	}