From 1805ab94feefb39c9545d99bd2a28b2f7408e54d Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 20 Sep 2020 10:07:16 +0800 Subject: [PATCH] change behavior when add repo with same name and url. Signed-off-by: yxxhero --- cmd/helm/repo_add.go | 4 ++ cmd/helm/repo_add_test.go | 19 ++++++++- pkg/repo/repo.go | 16 +++++++ pkg/repo/repo_test.go | 89 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 1 deletion(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 1c2162bfa..ff00a30ac 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -115,6 +115,10 @@ func (o *repoAddOptions) run(out io.Writer) error { if err := yaml.Unmarshal(b, &f); err != nil { return err } + // If the repo with specified name and url is already exists, directory returun. + if f.HasRepoWithNameAndURL(o.name, o.url) { + return nil + } // If the repo exists and --force-update was not specified, error out. if !o.forceUpdate && f.Has(o.name) { diff --git a/cmd/helm/repo_add_test.go b/cmd/helm/repo_add_test.go index d358ad970..d99d13248 100644 --- a/cmd/helm/repo_add_test.go +++ b/cmd/helm/repo_add_test.go @@ -59,6 +59,12 @@ func TestRepoAdd(t *testing.T) { } defer ts.Stop() + tsn, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") + if err != nil { + t.Fatal(err) + } + defer tsn.Stop() + rootDir := ensure.TempDir(t) repoFile := filepath.Join(rootDir, "repositories.yaml") @@ -77,12 +83,17 @@ func TestRepoAdd(t *testing.T) { t.Error(err) } + // add repo with same name and url is ok. + if err := o.run(ioutil.Discard); err != nil { + t.Error(err) + } + f, err := repo.LoadFile(repoFile) if err != nil { t.Fatal(err) } - if !f.Has(testRepoName) { + if !f.HasRepoWithNameAndURL(testRepoName, ts.URL()) { t.Errorf("%s was not successfully inserted into %s", testRepoName, repoFile) } @@ -95,6 +106,12 @@ func TestRepoAdd(t *testing.T) { t.Errorf("Error cache charts file was not created for repository %s", testRepoName) } + o.url = tsn.URL() + + if err := o.run(ioutil.Discard); err == nil { + t.Errorf("Expected error: repo is already exists with different url, but got nil") + } + o.forceUpdate = true if err := o.run(ioutil.Discard); err != nil { diff --git a/pkg/repo/repo.go b/pkg/repo/repo.go index 6f1e90dad..7a1bc713a 100644 --- a/pkg/repo/repo.go +++ b/pkg/repo/repo.go @@ -85,6 +85,22 @@ func (r *File) Has(name string) bool { return entry != nil } +// HasRepoWithNameAndURL returns true if the given name whth url is already a repository name. +func (r *File) HasRepoWithNameAndURL(name string, url string) bool { + entry := r.GetRepoByNameAndURL(name, url) + return entry != nil +} + +// GetRepoByNameAndURL returns an entry with the given name and url if it exists, otherwise returns nil +func (r *File) GetRepoByNameAndURL(name string, url string) *Entry { + for _, entry := range r.Repositories { + if entry.Name == name && entry.URL == url { + return entry + } + } + return nil +} + // Get returns an entry with the given name if it exists, otherwise returns nil func (r *File) Get(name string) *Entry { for _, entry := range r.Repositories { diff --git a/pkg/repo/repo_test.go b/pkg/repo/repo_test.go index f87d2c202..a13af423a 100644 --- a/pkg/repo/repo_test.go +++ b/pkg/repo/repo_test.go @@ -90,6 +90,95 @@ func TestNewFile(t *testing.T) { } } } +func TestRepoFile_HasRepoWithNameAndURL(t *testing.T) { + repo := NewFile() + repo.Add( + &Entry{ + Name: "first", + URL: "https://example.com/first", + }, + &Entry{ + Name: "second", + URL: "https://example.com/second", + }, + &Entry{ + Name: "third", + URL: "https://example.com/third", + }, + &Entry{ + Name: "fourth", + URL: "https://example.com/fourth", + }, + ) + + name := "second" + url := "https://example.com/second" + + found := repo.HasRepoWithNameAndURL(name, url) + if !found { + t.Fatalf("Expected repo entry %q with name %q to be found", name, url) + } + + found = repo.HasRepoWithNameAndURL("nonexistent", "nonexistent") + if found { + t.Errorf("Got unexpected entry.") + } + + found = repo.HasRepoWithNameAndURL("first", "nonexistent") + if found { + t.Errorf("Got unexpected entry.") + } + + found = repo.HasRepoWithNameAndURL("nonexistent", "https://example.com/first") + if found { + t.Errorf("Got unexpected entry.") + } +} +func TestRepoFile_GetRepoByNameAndURL(t *testing.T) { + repo := NewFile() + repo.Add( + &Entry{ + Name: "first", + URL: "https://example.com/first", + }, + &Entry{ + Name: "second", + URL: "https://example.com/second", + }, + &Entry{ + Name: "third", + URL: "https://example.com/third", + }, + &Entry{ + Name: "fourth", + URL: "https://example.com/fourth", + }, + ) + + name := "second" + url := "https://example.com/second" + + entry := repo.GetRepoByNameAndURL(name, url) + if entry == nil { + t.Fatalf("Expected repo entry %q with name %q to be found", name, url) + } + + entry = repo.GetRepoByNameAndURL("nonexistent", "nonexistent") + if entry != nil { + t.Errorf("Got unexpected entry %+v", entry) + } + + entry = repo.GetRepoByNameAndURL("first", "nonexistent") + if entry != nil { + t.Errorf("Got unexpected entry %+v", entry) + } + + entry = repo.GetRepoByNameAndURL("nonexistent", "https://example.com/first") + if entry != nil { + t.Errorf("Got unexpected entry %+v", entry) + } + +} func TestRepoFile_Get(t *testing.T) { repo := NewFile()