From c86a8cbd53cacc6ef127d45122cfade0b50c0c0d Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Mon, 4 Jul 2022 18:43:29 -0300 Subject: [PATCH 1/5] perf(dep-up): do not update the same repo multiple times Signed-off-by: Felipe Santos --- pkg/downloader/manager.go | 18 +++++++++ pkg/downloader/manager_test.go | 69 ++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index a5b0af080..d0f101dab 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -659,9 +659,27 @@ func (m *Manager) UpdateRepositories() error { return nil } +// Filter out duplicate repos by URL, including those with trailing slashes. +func dedupeRepos(repos []*repo.Entry) []*repo.Entry { + seen := make(map[string]*repo.Entry) + for _, r := range repos { + // Normalize URL by removing trailing slashes. + r.URL = strings.TrimSuffix(r.URL, "/") + seen[r.URL] = r + } + var unique []*repo.Entry + for _, r := range seen { + unique = append(unique, r) + } + return unique +} + func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error { var wg sync.WaitGroup + + repos = dedupeRepos(repos) + for _, c := range repos { r, err := repo.NewChartRepository(c, m.Getters) if err != nil { diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index f7ab1a568..13c94e116 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -26,6 +26,7 @@ import ( "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/repo" "helm.sh/helm/v3/pkg/repo/repotest" ) @@ -572,3 +573,71 @@ func TestKey(t *testing.T) { } } } + +// Test dedupeRepos tests that the dedupeRepos function correctly deduplicates +func TestDedupeRepos(t *testing.T) { + tests := []struct { + name string + repos []*repo.Entry + want []*repo.Entry + }{ + { + name: "no duplicates", + repos: []*repo.Entry{ + { + URL: "https://example.com/charts", + }, + { + URL: "https://example.com/charts2", + }, + }, + want: []*repo.Entry{ + { + URL: "https://example.com/charts", + }, + { + URL: "https://example.com/charts2", + }, + }, + }, + { + name: "duplicates", + repos: []*repo.Entry{ + { + URL: "https://example.com/charts", + }, + { + URL: "https://example.com/charts", + }, + }, + want: []*repo.Entry{ + { + URL: "https://example.com/charts", + }, + }, + }, + { + name: "duplicates with trailing slash", + repos: []*repo.Entry{ + { + URL: "https://example.com/charts", + }, + { + URL: "https://example.com/charts/", + }, + }, + want: []*repo.Entry{ + { + URL: "https://example.com/charts", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := dedupeRepos(tt.repos); !reflect.DeepEqual(got, tt.want) { + t.Errorf("received:\n%v\nwant:\n%v", got, tt.want) + } + }) + } +} From bb6314adefd2020cf8adb45a3109e1957446b741 Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Thu, 20 Feb 2025 14:42:48 -0300 Subject: [PATCH 2/5] Do not reassign repos variable Signed-off-by: Felipe Santos --- pkg/downloader/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 2e284ab73..dfff0ddd4 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -678,9 +678,9 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error { var wg sync.WaitGroup - repos = dedupeRepos(repos) + localRepos := dedupeRepos(repos) - for _, c := range repos { + for _, c := range localRepos { r, err := repo.NewChartRepository(c, m.Getters) if err != nil { return err From 281ccb083543beeac21b9b72be9a07dbdd18f21f Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Thu, 20 Feb 2025 15:33:52 -0300 Subject: [PATCH 3/5] Do not store the normalized chart url Signed-off-by: Felipe Santos --- pkg/downloader/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index dfff0ddd4..c430eddaf 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -664,8 +664,8 @@ func dedupeRepos(repos []*repo.Entry) []*repo.Entry { seen := make(map[string]*repo.Entry) for _, r := range repos { // Normalize URL by removing trailing slashes. - r.URL = strings.TrimSuffix(r.URL, "/") - seen[r.URL] = r + seenUrl := strings.TrimRight(r.URL, "/") + seen[seenUrl] = r } var unique []*repo.Entry for _, r := range seen { From ecb5a2c9dd3f6363c7b153e7b3527db4444ff54f Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Thu, 20 Feb 2025 15:45:57 -0300 Subject: [PATCH 4/5] Fix variable name for linter And restore trimSuffix instead of trimRight, which was a mistake. Signed-off-by: Felipe Santos --- pkg/downloader/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index c430eddaf..52f7d5a92 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -664,8 +664,8 @@ func dedupeRepos(repos []*repo.Entry) []*repo.Entry { seen := make(map[string]*repo.Entry) for _, r := range repos { // Normalize URL by removing trailing slashes. - seenUrl := strings.TrimRight(r.URL, "/") - seen[seenUrl] = r + seenURL := strings.TrimSuffix(r.URL, "/") + seen[seenURL] = r } var unique []*repo.Entry for _, r := range seen { From b1fd2391679258afeb06bf6ad540c51eeb18f4ea Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Thu, 20 Feb 2025 16:58:55 -0300 Subject: [PATCH 5/5] Fix tests failing after removing repo normalization Signed-off-by: Felipe Santos --- pkg/downloader/manager_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 6a963f603..1c45ee011 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -649,7 +649,8 @@ func TestDedupeRepos(t *testing.T) { }, want: []*repo.Entry{ { - URL: "https://example.com/charts", + // the last one wins + URL: "https://example.com/charts/", }, }, },