From dd8d409a7458f20dea425eabf7865a723c856e3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Marc=C3=A9?= Date: Wed, 16 Oct 2024 19:36:33 +0200 Subject: [PATCH 1/8] test Allow same deps with same name but different repo. related #7413 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarek Marcé --- internal/resolver/resolver_test.go | 15 ++++++++++++++- ...charts-index.yaml => repository-1-index.yaml} | 0 .../testdata/repository/repository-2-index.yaml | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) rename internal/resolver/testdata/repository/{kubernetes-charts-index.yaml => repository-1-index.yaml} (100%) create mode 100644 internal/resolver/testdata/repository/repository-2-index.yaml diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 1e33837a9..11b636d60 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -137,9 +137,22 @@ func TestResolve(t *testing.T) { }, err: true, }, + { + name: "charts with same name but different repo", + req: []*chart.Dependency{ + {Name: "alpine", Repository: "repository-1", Version: "0.1.0"}, + {Name: "alpine", Repository: "repository-2", Version: "0.3.0", Alias: "alpine-alias"}, + }, + expect: &chart.Lock{ + Dependencies: []*chart.Dependency{ + {Name: "alpine", Repository: "repository-1", Version: "0.1.0"}, + {Name: "alpine", Repository: "repository-2", Version: "0.3.0"}, + }, + }, + }, } - repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"} + repoNames := map[string]string{"alpine": "repository-1", "redis": "repository-1", "alpine-alias": "repository-2"} registryClient, _ := registry.NewClient() r := New("testdata/chartpath", "testdata/repository", registryClient) for _, tt := range tests { diff --git a/internal/resolver/testdata/repository/kubernetes-charts-index.yaml b/internal/resolver/testdata/repository/repository-1-index.yaml similarity index 100% rename from internal/resolver/testdata/repository/kubernetes-charts-index.yaml rename to internal/resolver/testdata/repository/repository-1-index.yaml diff --git a/internal/resolver/testdata/repository/repository-2-index.yaml b/internal/resolver/testdata/repository/repository-2-index.yaml new file mode 100644 index 000000000..daa3591bb --- /dev/null +++ b/internal/resolver/testdata/repository/repository-2-index.yaml @@ -0,0 +1,16 @@ +apiVersion: v1 +entries: + alpine: + - name: alpine + urls: + - https://charts.helm.sh/stable/alpine-0.1.0.tgz + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + home: https://helm.sh/helm + sources: + - https://github.com/helm/helm + version: 0.3.0 + description: Deploy a basic Alpine Linux pod + keywords: [] + maintainers: [] + icon: "" + apiVersion: v2 From 6bd8277e2d1d034a5cc0ccd228199b3033a8c348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Marc=C3=A9?= Date: Wed, 16 Oct 2024 19:56:01 +0200 Subject: [PATCH 2/8] Allow same deps with same name but different repo. Fix #7413 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarek Marcé --- internal/resolver/resolver.go | 4 ++++ pkg/downloader/manager.go | 12 ++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 13dcd2ce9..4ebdb0801 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -108,6 +108,10 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string } repoName := repoNames[d.Name] + if d.Alias != "" { + repoName = repoNames[d.Alias] + } + // if the repository was not defined, but the dependency defines a repository url, bypass the cache if repoName == "" && d.Repository != "" { locked[i] = &chart.Dependency{ diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 348c78edb..eaaf00c1c 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -572,6 +572,10 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, // by Helm. missing := []string{} for _, dd := range deps { + chartName := dd.Name + if dd.Alias != "" { + chartName = dd.Alias + } // Don't map the repository, we don't need to download chart from charts directory if dd.Repository == "" { continue @@ -585,12 +589,12 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, if m.Debug { fmt.Fprintf(m.Out, "Repository from local path: %s\n", dd.Repository) } - reposMap[dd.Name] = dd.Repository + reposMap[chartName] = dd.Repository continue } if registry.IsOCI(dd.Repository) { - reposMap[dd.Name] = dd.Repository + reposMap[chartName] = dd.Repository continue } @@ -601,11 +605,11 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, (strings.HasPrefix(dd.Repository, "alias:") && strings.TrimPrefix(dd.Repository, "alias:") == repo.Name) { found = true dd.Repository = repo.URL - reposMap[dd.Name] = repo.Name + reposMap[chartName] = repo.Name break } else if urlutil.Equal(repo.URL, dd.Repository) { found = true - reposMap[dd.Name] = repo.Name + reposMap[chartName] = repo.Name break } } From 04fb6d55b1063dd06fe1630abbde1cd643eb2bea Mon Sep 17 00:00:00 2001 From: kerat Date: Sun, 5 Oct 2025 23:16:01 +0200 Subject: [PATCH 3/8] refactor: improve validation error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarek Marcé --- pkg/chart/v2/metadata.go | 6 +++++- pkg/chart/v2/metadata_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/chart/v2/metadata.go b/pkg/chart/v2/metadata.go index c46007863..60abcae91 100644 --- a/pkg/chart/v2/metadata.go +++ b/pkg/chart/v2/metadata.go @@ -144,7 +144,11 @@ func (md *Metadata) Validate() error { key = dependency.Alias } if dependencies[key] != nil { - return ValidationErrorf("more than one dependency with name or alias %q", key) + return ValidationErrorf( + "more than one dependency with name or alias %q. "+ + "Try using distinct aliases for dependencies with the same chart name.", + key, + ) } dependencies[key] = dependency } diff --git a/pkg/chart/v2/metadata_test.go b/pkg/chart/v2/metadata_test.go index 7892f0209..ffee78dfc 100644 --- a/pkg/chart/v2/metadata_test.go +++ b/pkg/chart/v2/metadata_test.go @@ -98,7 +98,7 @@ func TestValidate(t *testing.T) { {Name: "foo", Alias: ""}, }, }, - ValidationError("more than one dependency with name or alias \"foo\""), + ValidationError("more than one dependency with name or alias \"foo\". Try using distinct aliases for dependencies with the same chart name."), }, { "two dependencies with alias from second dependency shadowing first one", @@ -112,7 +112,7 @@ func TestValidate(t *testing.T) { {Name: "bar", Alias: "foo"}, }, }, - ValidationError("more than one dependency with name or alias \"foo\""), + ValidationError("more than one dependency with name or alias \"foo\". Try using distinct aliases for dependencies with the same chart name."), }, { // this case would make sense and could work in future versions of Helm, currently template rendering would @@ -128,7 +128,7 @@ func TestValidate(t *testing.T) { {Name: "foo", Alias: "", Version: "1.0.0"}, }, }, - ValidationError("more than one dependency with name or alias \"foo\""), + ValidationError("more than one dependency with name or alias \"foo\". Try using distinct aliases for dependencies with the same chart name."), }, { // this case would make sense and could work in future versions of Helm, currently template rendering would @@ -144,7 +144,7 @@ func TestValidate(t *testing.T) { {Name: "foo", Repository: "repo-1"}, }, }, - ValidationError("more than one dependency with name or alias \"foo\""), + ValidationError("more than one dependency with name or alias \"foo\". Try using distinct aliases for dependencies with the same chart name."), }, { "dependencies has nil", From 172429ae1dd661fc1056333944142377a9390000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Marc=C3=A9?= Date: Sun, 5 Oct 2025 23:41:35 +0200 Subject: [PATCH 4/8] refactor: in ensureMissingRepos, use the alias (when present) as the key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarek Marcé --- pkg/downloader/manager.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index b9b1cab03..d38937289 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -506,6 +506,10 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. var ru []*repo.Entry for _, dd := range deps { + chartName := dd.Name + if dd.Alias != "" { + chartName = dd.Alias + } // If the chart is in the local charts directory no repository needs // to be specified. @@ -514,7 +518,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. } // When the repoName for a dependency is known we can skip ensuring - if _, ok := repoNames[dd.Name]; ok { + if _, ok := repoNames[chartName]; ok { continue } @@ -530,7 +534,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. } rn = managerKeyPrefix + rn - repoNames[dd.Name] = rn + repoNames[chartName] = rn // Assuming the repository is generally available. For Helm managed // access controls the repository needs to be added through the user From 3ec1c4b8702c8a076ee776a88b9445d5f5d834b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Marc=C3=A9?= Date: Wed, 16 Oct 2024 19:36:33 +0200 Subject: [PATCH 5/8] test Allow same deps with same name but different repo. related #7413 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarek Marcé --- internal/resolver/resolver_test.go | 15 ++++++++++++++- ...charts-index.yaml => repository-1-index.yaml} | 0 .../testdata/repository/repository-2-index.yaml | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) rename internal/resolver/testdata/repository/{kubernetes-charts-index.yaml => repository-1-index.yaml} (100%) create mode 100644 internal/resolver/testdata/repository/repository-2-index.yaml diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 1e33837a9..11b636d60 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -137,9 +137,22 @@ func TestResolve(t *testing.T) { }, err: true, }, + { + name: "charts with same name but different repo", + req: []*chart.Dependency{ + {Name: "alpine", Repository: "repository-1", Version: "0.1.0"}, + {Name: "alpine", Repository: "repository-2", Version: "0.3.0", Alias: "alpine-alias"}, + }, + expect: &chart.Lock{ + Dependencies: []*chart.Dependency{ + {Name: "alpine", Repository: "repository-1", Version: "0.1.0"}, + {Name: "alpine", Repository: "repository-2", Version: "0.3.0"}, + }, + }, + }, } - repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"} + repoNames := map[string]string{"alpine": "repository-1", "redis": "repository-1", "alpine-alias": "repository-2"} registryClient, _ := registry.NewClient() r := New("testdata/chartpath", "testdata/repository", registryClient) for _, tt := range tests { diff --git a/internal/resolver/testdata/repository/kubernetes-charts-index.yaml b/internal/resolver/testdata/repository/repository-1-index.yaml similarity index 100% rename from internal/resolver/testdata/repository/kubernetes-charts-index.yaml rename to internal/resolver/testdata/repository/repository-1-index.yaml diff --git a/internal/resolver/testdata/repository/repository-2-index.yaml b/internal/resolver/testdata/repository/repository-2-index.yaml new file mode 100644 index 000000000..daa3591bb --- /dev/null +++ b/internal/resolver/testdata/repository/repository-2-index.yaml @@ -0,0 +1,16 @@ +apiVersion: v1 +entries: + alpine: + - name: alpine + urls: + - https://charts.helm.sh/stable/alpine-0.1.0.tgz + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + home: https://helm.sh/helm + sources: + - https://github.com/helm/helm + version: 0.3.0 + description: Deploy a basic Alpine Linux pod + keywords: [] + maintainers: [] + icon: "" + apiVersion: v2 From a24b4e60f2697cd90cebe8433597745c8b0dfcec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Marc=C3=A9?= Date: Wed, 16 Oct 2024 19:56:01 +0200 Subject: [PATCH 6/8] Allow same deps with same name but different repo. Fix #7413 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarek Marcé --- internal/resolver/resolver.go | 4 ++++ pkg/downloader/manager.go | 12 ++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 3efe94f10..6875fb7cf 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -108,6 +108,10 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string } repoName := repoNames[d.Name] + if d.Alias != "" { + repoName = repoNames[d.Alias] + } + // if the repository was not defined, but the dependency defines a repository url, bypass the cache if repoName == "" && d.Repository != "" { locked[i] = &chart.Dependency{ diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 6043fbaaa..7ec45f0a2 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -576,6 +576,10 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, // by Helm. missing := []string{} for _, dd := range deps { + chartName := dd.Name + if dd.Alias != "" { + chartName = dd.Alias + } // Don't map the repository, we don't need to download chart from charts directory if dd.Repository == "" { continue @@ -589,12 +593,12 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, if m.Debug { fmt.Fprintf(m.Out, "Repository from local path: %s\n", dd.Repository) } - reposMap[dd.Name] = dd.Repository + reposMap[chartName] = dd.Repository continue } if registry.IsOCI(dd.Repository) { - reposMap[dd.Name] = dd.Repository + reposMap[chartName] = dd.Repository continue } @@ -605,11 +609,11 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, (strings.HasPrefix(dd.Repository, "alias:") && strings.TrimPrefix(dd.Repository, "alias:") == repo.Name) { found = true dd.Repository = repo.URL - reposMap[dd.Name] = repo.Name + reposMap[chartName] = repo.Name break } else if urlutil.Equal(repo.URL, dd.Repository) { found = true - reposMap[dd.Name] = repo.Name + reposMap[chartName] = repo.Name break } } From dc00c705d5fdccd601917d2500dbc4cecbb707fb Mon Sep 17 00:00:00 2001 From: kerat Date: Sun, 5 Oct 2025 23:16:01 +0200 Subject: [PATCH 7/8] refactor: improve validation error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarek Marcé --- pkg/chart/v2/metadata.go | 6 +++++- pkg/chart/v2/metadata_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/chart/v2/metadata.go b/pkg/chart/v2/metadata.go index c46007863..60abcae91 100644 --- a/pkg/chart/v2/metadata.go +++ b/pkg/chart/v2/metadata.go @@ -144,7 +144,11 @@ func (md *Metadata) Validate() error { key = dependency.Alias } if dependencies[key] != nil { - return ValidationErrorf("more than one dependency with name or alias %q", key) + return ValidationErrorf( + "more than one dependency with name or alias %q. "+ + "Try using distinct aliases for dependencies with the same chart name.", + key, + ) } dependencies[key] = dependency } diff --git a/pkg/chart/v2/metadata_test.go b/pkg/chart/v2/metadata_test.go index 7892f0209..ffee78dfc 100644 --- a/pkg/chart/v2/metadata_test.go +++ b/pkg/chart/v2/metadata_test.go @@ -98,7 +98,7 @@ func TestValidate(t *testing.T) { {Name: "foo", Alias: ""}, }, }, - ValidationError("more than one dependency with name or alias \"foo\""), + ValidationError("more than one dependency with name or alias \"foo\". Try using distinct aliases for dependencies with the same chart name."), }, { "two dependencies with alias from second dependency shadowing first one", @@ -112,7 +112,7 @@ func TestValidate(t *testing.T) { {Name: "bar", Alias: "foo"}, }, }, - ValidationError("more than one dependency with name or alias \"foo\""), + ValidationError("more than one dependency with name or alias \"foo\". Try using distinct aliases for dependencies with the same chart name."), }, { // this case would make sense and could work in future versions of Helm, currently template rendering would @@ -128,7 +128,7 @@ func TestValidate(t *testing.T) { {Name: "foo", Alias: "", Version: "1.0.0"}, }, }, - ValidationError("more than one dependency with name or alias \"foo\""), + ValidationError("more than one dependency with name or alias \"foo\". Try using distinct aliases for dependencies with the same chart name."), }, { // this case would make sense and could work in future versions of Helm, currently template rendering would @@ -144,7 +144,7 @@ func TestValidate(t *testing.T) { {Name: "foo", Repository: "repo-1"}, }, }, - ValidationError("more than one dependency with name or alias \"foo\""), + ValidationError("more than one dependency with name or alias \"foo\". Try using distinct aliases for dependencies with the same chart name."), }, { "dependencies has nil", From 9fe2110f4235f2bf51b5a3a54526278e42f4cd1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Marc=C3=A9?= Date: Sun, 5 Oct 2025 23:41:35 +0200 Subject: [PATCH 8/8] refactor: in ensureMissingRepos, use the alias (when present) as the key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarek Marcé --- pkg/downloader/manager.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 7ec45f0a2..dd21e5cbb 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -506,6 +506,10 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. var ru []*repo.Entry for _, dd := range deps { + chartName := dd.Name + if dd.Alias != "" { + chartName = dd.Alias + } // If the chart is in the local charts directory no repository needs // to be specified. @@ -514,7 +518,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. } // When the repoName for a dependency is known we can skip ensuring - if _, ok := repoNames[dd.Name]; ok { + if _, ok := repoNames[chartName]; ok { continue } @@ -530,7 +534,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. } rn = managerKeyPrefix + rn - repoNames[dd.Name] = rn + repoNames[chartName] = rn // Assuming the repository is generally available. For Helm managed // access controls the repository needs to be added through the user