From 53797863ad45b24a30bf42d9954ac4ae6b7341fb Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Tue, 1 Jun 2021 14:58:31 +0200 Subject: [PATCH] fix(helm): helm pull --repo to use repo creds from helm repos file Up to this point, helm pull --repo required --username and --password even if the repository had already been added via helm repo add. From now on, check repositories in the repositories file and if the URL matches, pull the chart with the username and password from the entry. Fixes #9599 Co-authored-by: Andreas Karis Co-authored-by: Felipe Santos Signed-off-by: Andreas Karis --- pkg/action/pull.go | 16 ++++++++++ pkg/cmd/pull_test.go | 69 ++++++++++++++++++++++++++++++++++++++++ pkg/repo/v1/repo.go | 34 ++++++++++++++++++++ pkg/repo/v1/repo_test.go | 43 +++++++++++++++++++++++++ 4 files changed, 162 insertions(+) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index dd051167b..b9a67a07a 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -18,6 +18,7 @@ package action import ( "fmt" + "log/slog" "os" "path/filepath" "strings" @@ -73,6 +74,21 @@ func (p *Pull) SetRegistryClient(client *registry.Client) { func (p *Pull) Run(chartRef string) (string, error) { var out strings.Builder + // If the repository URL is set via CLI (--repo), but neither a username nor + // password are set for a pull request, check if the repository credentials + // are already stored in helm's repository config. + if p.RepoURL != "" && p.Username == "" && p.Password == "" { + f, err := repo.LoadFile(p.Settings.RepositoryConfig) + if err != nil { + return out.String(), fmt.Errorf("failed to load repository config %q: %w", p.Settings.RepositoryConfig, err) + } + if repoEntry := f.GetByURL(p.RepoURL); repoEntry != nil { + slog.Debug("Using credentials from repository config") + p.Username = repoEntry.Username + p.Password = repoEntry.Password + } + } + c := downloader.ChartDownloader{ Out: &out, Keyring: p.Keyring, diff --git a/pkg/cmd/pull_test.go b/pkg/cmd/pull_test.go index 96631fe05..2e30a0da2 100644 --- a/pkg/cmd/pull_test.go +++ b/pkg/cmd/pull_test.go @@ -506,3 +506,72 @@ func TestPullFileCompletion(t *testing.T) { checkFileCompletion(t, "pull", false) checkFileCompletion(t, "pull repo/chart", false) } + +func TestPullWithCredentialsFromExistingRepository(t *testing.T) { + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz"), + repotest.WithMiddleware(repotest.BasicAuthMiddleware(t)), + ) + defer srv.Stop() + + srv2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.FileServer(http.Dir(srv.Root())).ServeHTTP(w, r) + })) + defer srv2.Close() + + if err := srv.LinkIndices(); err != nil { + t.Fatal(err) + } + + // all flags will get "-d outdir" appended. + tests := []struct { + name string + args string + expectFile string + }{ + { + name: "Chart fetch using repo URL", + expectFile: "./signtest-0.1.0.tgz", + args: "signtest --repo " + srv.URL() + " --username username --password password", + }, + { + name: "Chart fetch using repo URL", + expectFile: "./signtest-0.1.0.tgz", + args: "signtest --repo " + srv.URL(), + }, + } + + outdir := t.TempDir() + repositoryConfig := filepath.Join(outdir, "repositories.yaml") + repositoryCache := outdir + registryConfig := filepath.Join(outdir, "config.json") + cmd := fmt.Sprintf("repo add test_repository %s --username %s --password %s "+ + "--repository-config %s --repository-cache %s --registry-config %s", + srv.URL(), "username", "password", repositoryConfig, repositoryCache, registryConfig, + ) + if _, _, err := executeActionCommand(cmd); err != nil { + t.Fatalf("got error in setup, error: %q", err) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := fmt.Sprintf("pull %s -d '%s' --repository-config %s --repository-cache %s --registry-config %s", + tt.args, + outdir, + repositoryConfig, + repositoryCache, + registryConfig, + ) + _, _, err := executeActionCommand(cmd) + if err != nil { + t.Fatalf("%q reported error: %s", tt.name, err) + } + + ef := filepath.Join(outdir, tt.expectFile) + if _, err := os.Stat(ef); err != nil { + t.Errorf("%q: expected a file at %s. %s", tt.name, ef, err) + } + }) + } +} diff --git a/pkg/repo/v1/repo.go b/pkg/repo/v1/repo.go index 38d2b0ca1..06b33229a 100644 --- a/pkg/repo/v1/repo.go +++ b/pkg/repo/v1/repo.go @@ -18,7 +18,9 @@ package repo // import "helm.sh/helm/v4/pkg/repo/v1" import ( "fmt" + "net/url" "os" + "path" "path/filepath" "time" @@ -94,6 +96,38 @@ func (r *File) Get(name string) *Entry { return nil } +// compareURLs returns true if url1 and url2 can each be parsed, have the exact +// same scheme and host and the same cleaned path. +func compareURLs(url1, url2 string) bool { + parsed1, err := url.Parse(url1) + if err != nil { + return false + } + parsed2, err := url.Parse(url2) + if err != nil { + return false + } + if parsed1.Scheme != parsed2.Scheme { + return false + } + if parsed1.Host != parsed2.Host { + return false + } + return path.Clean(parsed1.Path) == path.Clean(parsed2.Path) +} + +// GetByURL returns the first entry with the given URL if one exists, otherwise +// returns nil. For comparison, GetByURL tries to sanitize/standardize the URLs +// as much as possible. +func (r *File) GetByURL(url string) *Entry { + for _, entry := range r.Repositories { + if compareURLs(entry.URL, url) { + return entry + } + } + return nil +} + // Remove removes the entry from the list of repositories. func (r *File) Remove(name string) bool { cp := []*Entry{} diff --git a/pkg/repo/v1/repo_test.go b/pkg/repo/v1/repo_test.go index bdaa61eda..68039b849 100644 --- a/pkg/repo/v1/repo_test.go +++ b/pkg/repo/v1/repo_test.go @@ -128,6 +128,49 @@ func TestRepoFile_Get(t *testing.T) { } } +func TestRepoFileGetByURL(t *testing.T) { + repo := NewFile() + repo.Add( + &Entry{ + Name: "first", + URL: "https://example.com/first", + }, + &Entry{ + Name: "second1", + URL: "https://example.comm/second", + }, + &Entry{ + Name: "second2", + URL: "http://example.com/second", + }, + &Entry{ + Name: "second3", + URL: "https://example.com/second/////////", + }, + &Entry{ + Name: "third", + URL: "https://example.com/third", + }, + ) + + url := "https://example.com/second" + + entry := repo.GetByURL(url) + if entry == nil { + t.Fatalf("Expected repo entry %q to be found", url) + } + + expected := "second3" + if entry.Name != expected { + t.Errorf("Expected repo Name to be %q but got %q", expected, entry.Name) + } + + entry = repo.GetByURL("http://nonexistent.example.com/nonexistent") + if entry != nil { + t.Errorf("Got unexpected entry %+v", entry) + } +} + func TestRemoveRepository(t *testing.T) { sampleRepository := NewFile() sampleRepository.Add(