From 8c3aae1ee6e273d9dad07dbf380f35b9923c4f34 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 | 18 +++++++++++ pkg/cmd/pull_test.go | 64 ++++++++++++++++++++++++++++++++++++++++ pkg/repo/v1/repo.go | 42 ++++++++++++++++++++++++++ pkg/repo/v1/repo_test.go | 43 +++++++++++++++++++++++++++ 4 files changed, 167 insertions(+) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index dd051167b..89824fc4e 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -73,6 +73,8 @@ func (p *Pull) SetRegistryClient(client *registry.Client) { func (p *Pull) Run(chartRef string) (string, error) { var out strings.Builder + p.loadMissingCredentials() + c := downloader.ChartDownloader{ Out: &out, Keyring: p.Keyring, @@ -173,3 +175,19 @@ func (p *Pull) Run(chartRef string) (string, error) { } return out.String(), nil } + +// loadMissingCredentials checks if the repository credentials are already stored in helm's repository config, if the +// repository URL is set via CLI (--repo), but neither a username nor password are set for a pull request. +func (p *Pull) loadMissingCredentials() { + if p.RepoURL != "" && p.Username == "" && p.Password == "" { + f, err := repo.LoadFile(p.Settings.RepositoryConfig) + if err != nil { + return + } + if repoEntry := f.GetByURL(p.RepoURL); repoEntry != nil { + p.cfg.Logger().Debug("Using credentials from repository config") + p.Username = repoEntry.Username + p.Password = repoEntry.Password + } + } +} diff --git a/pkg/cmd/pull_test.go b/pkg/cmd/pull_test.go index f749c218c..ac1ea91fe 100644 --- a/pkg/cmd/pull_test.go +++ b/pkg/cmd/pull_test.go @@ -558,3 +558,67 @@ func TestPullOCIWithTagAndDigest(t *testing.T) { } } } + +func TestPullWithCredentialsFromExistingRepository(t *testing.T) { + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz"), + repotest.WithMiddleware(repotest.BasicAuthMiddleware(t)), + ) + defer srv.Stop() + + 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 with username and password", + expectFile: "./signtest-0.1.0.tgz", + args: "signtest --repo " + srv.URL() + " --username username --password password", + }, + { + name: "Chart fetch using repo URL with stored credentials", + 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..970289bf8 100644 --- a/pkg/repo/v1/repo.go +++ b/pkg/repo/v1/repo.go @@ -18,8 +18,11 @@ package repo // import "helm.sh/helm/v4/pkg/repo/v1" import ( "fmt" + "net/url" "os" + "path" "path/filepath" + "strings" "time" "sigs.k8s.io/yaml" @@ -94,6 +97,45 @@ 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 !strings.EqualFold(parsed1.Scheme, parsed2.Scheme) { + return false + } + if !strings.EqualFold(parsed1.Host, parsed2.Host) { + return false + } + return path.Clean(normalizePath(parsed1.Path)) == path.Clean(normalizePath(parsed2.Path)) +} + +func normalizePath(path string) string { + if path == "" { + return "/" + } + return 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 9b5c54309..57f3e52da 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(