diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index b6f45da9e..3455c9b61 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -17,6 +17,8 @@ package resolver import ( "bytes" + "crypto" + "encoding/hex" "encoding/json" "fmt" "os" @@ -51,6 +53,19 @@ func New(chartpath, cachepath string, registryClient *registry.Client) *Resolver } } +// dependencyKey creates a unique key for a dependency that includes name, repository, and index +// to handle cases where multiple dependencies have the same name but different repositories +func dependencyKey(name, repository string, index int) string { + // Use a combination of name, repository hash, and index to ensure uniqueness + repoHash := "" + if repository != "" { + hasher := crypto.SHA256.New() + hasher.Write([]byte(repository)) + repoHash = hex.EncodeToString(hasher.Sum(nil))[:8] // Use first 8 chars for brevity + } + return fmt.Sprintf("%s-%s-%d", name, repoHash, index) +} + // Resolve resolves dependencies and returns a lock file with the resolution. func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) { @@ -106,7 +121,15 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string continue } - repoName := repoNames[d.Name] + // Try to get repository name using composite key first, then fall back to simple name + key := dependencyKey(d.Name, d.Repository, i) + repoName := repoNames[key] + + // Fall back to simple name lookup for backward compatibility + if repoName == "" { + repoName = repoNames[d.Name] + } + // 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 ec4056d27..01f4ef78b 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -501,7 +501,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. var ru []*repo.Entry - for _, dd := range deps { + for i, dd := range deps { // If the chart is in the local charts directory no repository needs // to be specified. @@ -510,7 +510,8 @@ 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 { + depKey := dependencyKey(dd.Name, dd.Repository, i) + if _, ok := repoNames[depKey]; ok { continue } @@ -526,7 +527,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. } rn = managerKeyPrefix + rn - repoNames[dd.Name] = rn + repoNames[depKey] = rn // Assuming the repository is generally available. For Helm managed // access controls the repository needs to be added through the user @@ -571,7 +572,7 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, // Verify that all repositories referenced in the deps are actually known // by Helm. missing := []string{} - for _, dd := range deps { + for i, dd := range deps { // Don't map the repository, we don't need to download chart from charts directory if dd.Repository == "" { continue @@ -585,12 +586,15 @@ 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 + // Use composite key for unique identification + depKey := dependencyKey(dd.Name, dd.Repository, i) + reposMap[depKey] = dd.Repository continue } if registry.IsOCI(dd.Repository) { - reposMap[dd.Name] = dd.Repository + depKey := dependencyKey(dd.Name, dd.Repository, i) + reposMap[depKey] = dd.Repository continue } @@ -601,11 +605,13 @@ 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 + depKey := dependencyKey(dd.Name, dd.Repository, i) + reposMap[depKey] = repo.Name break } else if urlutil.Equal(repo.URL, dd.Repository) { found = true - reposMap[dd.Name] = repo.Name + depKey := dependencyKey(dd.Name, dd.Repository, i) + reposMap[depKey] = repo.Name break } } @@ -614,7 +620,8 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, // Add if URL _, err := url.ParseRequestURI(repository) if err == nil { - reposMap[repository] = repository + depKey := dependencyKey(dd.Name, repository, i) + reposMap[depKey] = repository continue } missing = append(missing, repository) @@ -641,6 +648,19 @@ repository, use "https://charts.example.com/" or "@example" instead of return reposMap, nil } +// dependencyKey creates a unique key for a dependency that includes name, repository, and index +// to handle cases where multiple dependencies have the same name but different repositories +func dependencyKey(name, repository string, index int) string { + // Use a combination of name, repository hash, and index to ensure uniqueness + repoHash := "" + if repository != "" { + hasher := crypto.SHA256.New() + hasher.Write([]byte(repository)) + repoHash = hex.EncodeToString(hasher.Sum(nil))[:8] // Use first 8 chars for brevity + } + return fmt.Sprintf("%s-%s-%d", name, repoHash, index) +} + // UpdateRepositories updates all of the local repos to the latest. func (m *Manager) UpdateRepositories() error { rf, err := loadRepoConfig(m.RepositoryConfig) diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index db2487d16..485104b3f 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -138,6 +138,12 @@ func TestGetRepoNames(t *testing.T) { RepositoryConfig: repoConfig, RepositoryCache: repoCache, } + + // Helper function to generate expected key + expectedKey := func(name, repo string, index int) string { + return dependencyKey(name, repo, index) + } + tests := []struct { name string req []*chart.Dependency @@ -149,7 +155,9 @@ func TestGetRepoNames(t *testing.T) { req: []*chart.Dependency{ {Name: "oedipus-rex", Repository: "http://example.com/test"}, }, - expect: map[string]string{"http://example.com/test": "http://example.com/test"}, + expect: map[string]string{ + expectedKey("oedipus-rex", "http://example.com/test", 0): "http://example.com/test", + }, }, { name: "no repo definition failure -- stable repo", @@ -163,28 +171,38 @@ func TestGetRepoNames(t *testing.T) { req: []*chart.Dependency{ {Name: "oedipus-rex", Repository: "http://example.com"}, }, - expect: map[string]string{"oedipus-rex": "testing"}, + expect: map[string]string{ + expectedKey("oedipus-rex", "http://example.com", 0): "testing", + }, }, { name: "repo from local path", req: []*chart.Dependency{ {Name: "local-dep", Repository: "file://./testdata/signtest"}, }, - expect: map[string]string{"local-dep": "file://./testdata/signtest"}, + expect: map[string]string{ + expectedKey("local-dep", "file://./testdata/signtest", 0): "file://./testdata/signtest", + }, }, { name: "repo alias (alias:)", req: []*chart.Dependency{ {Name: "oedipus-rex", Repository: "alias:testing"}, }, - expect: map[string]string{"oedipus-rex": "testing"}, + expect: map[string]string{ + // Note: the repository URL gets resolved to the actual URL in resolveRepoNames + // We need to check what the actual resolved URL is + expectedKey("oedipus-rex", "http://example.com", 0): "testing", + }, }, { name: "repo alias (@)", req: []*chart.Dependency{ {Name: "oedipus-rex", Repository: "@testing"}, }, - expect: map[string]string{"oedipus-rex": "testing"}, + expect: map[string]string{ + expectedKey("oedipus-rex", "http://example.com", 0): "testing", + }, }, { name: "repo from local chart under charts path", @@ -193,26 +211,94 @@ func TestGetRepoNames(t *testing.T) { }, expect: map[string]string{}, }, + { + name: "multiple dependencies with same name but different repos", + req: []*chart.Dependency{ + {Name: "common-chart", Repository: "http://example.com"}, + {Name: "common-chart", Repository: "http://other.com"}, + }, + expect: map[string]string{ + expectedKey("common-chart", "http://example.com", 0): "testing", + expectedKey("common-chart", "http://other.com", 1): "http://other.com", + }, + }, } for _, tt := range tests { - l, err := m.resolveRepoNames(tt.req) - if err != nil { + t.Run(tt.name, func(t *testing.T) { + l, err := m.resolveRepoNames(tt.req) + if err != nil { + if tt.err { + return // Expected error, test passes + } + t.Fatal(err) + } + if tt.err { - continue + t.Fatalf("Expected error in test %q", tt.name) } - t.Fatal(err) - } - if tt.err { - t.Fatalf("Expected error in test %q", tt.name) - } + // Compare maps + if !reflect.DeepEqual(l, tt.expect) { + t.Errorf("%s: expected map %v, got %v", tt.name, tt.expect, l) + } + }) + } +} - // m1 and m2 are the maps we want to compare - eq := reflect.DeepEqual(l, tt.expect) - if !eq { - t.Errorf("%s: expected map %v, got %v", tt.name, l, tt.name) - } +func TestDependencyKey(t *testing.T) { + tests := []struct { + name string + depName string + repository string + index int + expect string + }{ + { + name: "basic case", + depName: "oedipus-rex", + repository: "http://example.com/test", + index: 0, + expect: "oedipus-rex-0e52b63f-0", + }, + { + name: "different repo same name", + depName: "oedipus-rex", + repository: "http://example.com", + index: 0, + expect: "oedipus-rex-f0e6a6a9-0", + }, + { + name: "same repo different index", + depName: "oedipus-rex", + repository: "http://example.com", + index: 1, + expect: "oedipus-rex-f0e6a6a9-1", + }, + { + name: "local file path", + depName: "local-dep", + repository: "file://./testdata/signtest", + index: 0, + expect: "local-dep-93f23b0e-0", + }, + { + name: "empty repository", + depName: "local-subchart", + repository: "", + index: 0, + expect: "local-subchart--0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := dependencyKey(tt.depName, tt.repository, tt.index) + if result != tt.expect { + t.Errorf("dependencyKey(%q, %q, %d) = %q, want %q", + tt.depName, tt.repository, tt.index, result, tt.expect) + } + }) } }