From 3cde12ce9beb0ff9cd73b37ea63a50a85e744da8 Mon Sep 17 00:00:00 2001 From: Dominik Braun Date: Fri, 4 Dec 2020 21:23:22 +0100 Subject: [PATCH 1/7] Add Entry.URLWithTrailingSlash method for normalizing repo URLs Signed-off-by: Dominik Braun --- pkg/repo/chartrepo.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 92892bb85..3305bdf77 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -292,3 +292,13 @@ func (e *Entry) String() string { } return string(buf) } + +// URLWithTrailingSlash returns the repository URL with a trailing slash. +// If the URL already ends with a slash, it will be returned unchanged. +func (e *Entry) URLWithTrailingSlash() string { + if e.URL[len(e.URL)-1:] == "/" { + return e.URL + } + + return e.URL + "/" +} From f234d15730463b8a2a3191910025fdee7888a884 Mon Sep 17 00:00:00 2001 From: Dominik Braun Date: Fri, 4 Dec 2020 21:23:52 +0100 Subject: [PATCH 2/7] Add tests for Entry.URLWithTrailingSlash Signed-off-by: Dominik Braun --- pkg/repo/chartrepo_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index cb0a129a1..096f1b60d 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -400,3 +400,19 @@ func TestResolveReferenceURL(t *testing.T) { t.Errorf("%s", chartURL) } } + +func TestEntry_URLWithTrailingSlash(t *testing.T) { + urlWithTrailingSlash := "http://someserver/something/" + e := Entry{URL: urlWithTrailingSlash} + + if e.URLWithTrailingSlash() != urlWithTrailingSlash { + t.Errorf("Expected unchanged repository URL") + } + + urlWithoutTrailingSlash := strings.TrimSuffix(urlWithTrailingSlash, "/") + e = Entry{URL: urlWithoutTrailingSlash} + + if e.URLWithTrailingSlash() != urlWithTrailingSlash { + t.Errorf("Expected repository URL without trailing slash") + } +} From 2ed0081b92595f117c1864a99a9bfd4170a13d84 Mon Sep 17 00:00:00 2001 From: Dominik Braun Date: Fri, 4 Dec 2020 21:33:13 +0100 Subject: [PATCH 3/7] Only fail if the repository URLs are different Signed-off-by: Dominik Braun --- cmd/helm/repo_add.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 1b09ece83..4c3e909d3 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -161,10 +161,13 @@ func (o *repoAddOptions) run(out io.Writer) error { if !o.forceUpdate && f.Has(o.name) { existing := f.Get(o.name) if c != *existing { - - // The input coming in for the name is different from what is already - // configured. Return an error. - return errors.Errorf("repository name (%s) already exists, please specify a different name", o.name) + // In case of of the repo URLs ends with a trailing slash and the other + // one doesn't, the config is considered to be different even though they + // refer to the same repository. + // Only fail with an error if the configs actually are different. + if c.URLWithTrailingSlash() != existing.URLWithTrailingSlash() { + return errors.Errorf("repository name (%s) already exists, please specify a different name", o.name) + } } // The add is idempotent so do nothing From ea119d0dc58b0a999fe319390e98622f585ae3b8 Mon Sep 17 00:00:00 2001 From: Dominik Braun Date: Fri, 4 Dec 2020 23:15:45 +0100 Subject: [PATCH 4/7] Simplify Entry.URLWithTrailingSlash implementation and usage Signed-off-by: Dominik Braun --- cmd/helm/repo_add.go | 14 ++++++-------- pkg/repo/chartrepo.go | 6 +----- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 4c3e909d3..1e860aa27 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -160,14 +160,12 @@ func (o *repoAddOptions) run(out io.Writer) error { // 2. When the config is different require --force-update if !o.forceUpdate && f.Has(o.name) { existing := f.Get(o.name) - if c != *existing { - // In case of of the repo URLs ends with a trailing slash and the other - // one doesn't, the config is considered to be different even though they - // refer to the same repository. - // Only fail with an error if the configs actually are different. - if c.URLWithTrailingSlash() != existing.URLWithTrailingSlash() { - return errors.Errorf("repository name (%s) already exists, please specify a different name", o.name) - } + // In case of of the repo URLs ends with a trailing slash and the other + // one doesn't, the config is considered to be different even though they + // refer to the same repository. + // Only fail with an error if the configs actually are different. + if c != *existing && c.URLWithTrailingSlash() != existing.URLWithTrailingSlash() { + return errors.Errorf("repository name (%s) already exists, please specify a different name", o.name) } // The add is idempotent so do nothing diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 3305bdf77..5a434eff7 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -296,9 +296,5 @@ func (e *Entry) String() string { // URLWithTrailingSlash returns the repository URL with a trailing slash. // If the URL already ends with a slash, it will be returned unchanged. func (e *Entry) URLWithTrailingSlash() string { - if e.URL[len(e.URL)-1:] == "/" { - return e.URL - } - - return e.URL + "/" + return strings.TrimSuffix(e.URL, "/") + "/" } From 3e751445a85c042c3f5d7f9d0d1c15ee9cf7de3a Mon Sep 17 00:00:00 2001 From: Dominik Braun Date: Sat, 5 Dec 2020 15:25:58 +0100 Subject: [PATCH 5/7] Use correct error message for failing test Signed-off-by: Dominik Braun --- pkg/repo/chartrepo_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index 096f1b60d..bdd214b22 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -413,6 +413,6 @@ func TestEntry_URLWithTrailingSlash(t *testing.T) { e = Entry{URL: urlWithoutTrailingSlash} if e.URLWithTrailingSlash() != urlWithTrailingSlash { - t.Errorf("Expected repository URL without trailing slash") + t.Errorf("Expected repository URL with trailing slash") } } From d4c250f5085c85502b042eba4fd847ffbdf41aa0 Mon Sep 17 00:00:00 2001 From: Dominik Braun Date: Mon, 21 Dec 2020 09:37:16 +0100 Subject: [PATCH 6/7] Remove explicit comparison of entry URLs Signed-off-by: Dominik Braun --- cmd/helm/repo_add.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 1e860aa27..98ccfcd79 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -160,11 +160,10 @@ func (o *repoAddOptions) run(out io.Writer) error { // 2. When the config is different require --force-update if !o.forceUpdate && f.Has(o.name) { existing := f.Get(o.name) - // In case of of the repo URLs ends with a trailing slash and the other - // one doesn't, the config is considered to be different even though they - // refer to the same repository. - // Only fail with an error if the configs actually are different. - if c != *existing && c.URLWithTrailingSlash() != existing.URLWithTrailingSlash() { + existing.URL = existing.URLWithTrailingSlash() + c.URL = c.URLWithTrailingSlash() + // Only fail with an error if the configs are different. + if c != *existing { return errors.Errorf("repository name (%s) already exists, please specify a different name", o.name) } From 433c3d8d14f9184108f4bb5b4fca2c3a928196bf Mon Sep 17 00:00:00 2001 From: Dominik Braun Date: Fri, 29 Jan 2021 22:26:33 +0100 Subject: [PATCH 7/7] Don't overwrite repo entries when checking for trailing slashes Signed-off-by: Dominik Braun --- cmd/helm/repo_add.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 98ccfcd79..994a900a3 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -160,10 +160,8 @@ func (o *repoAddOptions) run(out io.Writer) error { // 2. When the config is different require --force-update if !o.forceUpdate && f.Has(o.name) { existing := f.Get(o.name) - existing.URL = existing.URLWithTrailingSlash() - c.URL = c.URLWithTrailingSlash() // Only fail with an error if the configs are different. - if c != *existing { + if c.URLWithTrailingSlash() != existing.URLWithTrailingSlash() { return errors.Errorf("repository name (%s) already exists, please specify a different name", o.name) }