From b1a5d7c04c21fa323c1feffda02b8ad132e59a70 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 | 19 +++++++++++- pkg/cmd/pull_test.go | 69 +++++++++++++++++++++++++++++++++++++++++++ pkg/repo/repo.go | 34 +++++++++++++++++++++ pkg/repo/repo_test.go | 43 +++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 1 deletion(-) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index a2f53af0d..25ac9d7aa 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,23 @@ 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 != "" { + if p.Username == "" && p.Password == "" { + f, err := repo.LoadFile(p.Settings.RepositoryConfig) + if err != nil { + return out.String(), err + } + if repoEntry := f.GetByURL(p.RepoURL); repoEntry != nil { + slog.Info("Using credentials from repository config") + p.Username = repoEntry.Username + p.Password = repoEntry.Password + } + } + } + c := downloader.ChartDownloader{ Out: &out, Keyring: p.Keyring, @@ -113,7 +131,6 @@ func (p *Pull) Run(chartRef string) (string, error) { } defer os.RemoveAll(dest) } - if p.RepoURL != "" { chartURL, err := repo.FindChartInRepoURL( p.RepoURL, diff --git a/pkg/cmd/pull_test.go b/pkg/cmd/pull_test.go index c30c94b49..75b41086c 100644 --- a/pkg/cmd/pull_test.go +++ b/pkg/cmd/pull_test.go @@ -393,3 +393,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/repo.go b/pkg/repo/repo.go index 48c0e0193..172905ca3 100644 --- a/pkg/repo/repo.go +++ b/pkg/repo/repo.go @@ -18,7 +18,9 @@ package repo // import "helm.sh/helm/v4/pkg/repo" 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/repo_test.go b/pkg/repo/repo_test.go index bdaa61eda..68039b849 100644 --- a/pkg/repo/repo_test.go +++ b/pkg/repo/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(