fix: resolve dependencies with same name from different repositories

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
pull/30946/head
Rick Brouwer 4 months ago
parent 0c91649ec8
commit 161da1131e
No known key found for this signature in database

@ -17,6 +17,8 @@ package resolver
import ( import (
"bytes" "bytes"
"crypto"
"encoding/hex"
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
@ -52,6 +54,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. // 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) { func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) {
@ -107,7 +122,15 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
continue 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 the repository was not defined, but the dependency defines a repository url, bypass the cache
if repoName == "" && d.Repository != "" { if repoName == "" && d.Repository != "" {
locked[i] = &chart.Dependency{ locked[i] = &chart.Dependency{

@ -501,7 +501,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.
var ru []*repo.Entry 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 // If the chart is in the local charts directory no repository needs
// to be specified. // 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 // 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 continue
} }
@ -526,7 +527,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.
} }
rn = managerKeyPrefix + rn rn = managerKeyPrefix + rn
repoNames[dd.Name] = rn repoNames[depKey] = rn
// Assuming the repository is generally available. For Helm managed // Assuming the repository is generally available. For Helm managed
// access controls the repository needs to be added through the user // 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 // Verify that all repositories referenced in the deps are actually known
// by Helm. // by Helm.
missing := []string{} 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 // Don't map the repository, we don't need to download chart from charts directory
if dd.Repository == "" { if dd.Repository == "" {
continue continue
@ -585,12 +586,15 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string,
if m.Debug { if m.Debug {
fmt.Fprintf(m.Out, "Repository from local path: %s\n", dd.Repository) 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 continue
} }
if registry.IsOCI(dd.Repository) { if registry.IsOCI(dd.Repository) {
reposMap[dd.Name] = dd.Repository depKey := dependencyKey(dd.Name, dd.Repository, i)
reposMap[depKey] = dd.Repository
continue 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) { (strings.HasPrefix(dd.Repository, "alias:") && strings.TrimPrefix(dd.Repository, "alias:") == repo.Name) {
found = true found = true
dd.Repository = repo.URL dd.Repository = repo.URL
reposMap[dd.Name] = repo.Name depKey := dependencyKey(dd.Name, dd.Repository, i)
reposMap[depKey] = repo.Name
break break
} else if urlutil.Equal(repo.URL, dd.Repository) { } else if urlutil.Equal(repo.URL, dd.Repository) {
found = true found = true
reposMap[dd.Name] = repo.Name depKey := dependencyKey(dd.Name, dd.Repository, i)
reposMap[depKey] = repo.Name
break break
} }
} }
@ -614,7 +620,8 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string,
// Add if URL // Add if URL
_, err := url.ParseRequestURI(repository) _, err := url.ParseRequestURI(repository)
if err == nil { if err == nil {
reposMap[repository] = repository depKey := dependencyKey(dd.Name, repository, i)
reposMap[depKey] = repository
continue continue
} }
missing = append(missing, repository) missing = append(missing, repository)
@ -641,6 +648,19 @@ repository, use "https://charts.example.com/" or "@example" instead of
return reposMap, nil 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. // UpdateRepositories updates all of the local repos to the latest.
func (m *Manager) UpdateRepositories() error { func (m *Manager) UpdateRepositories() error {
rf, err := loadRepoConfig(m.RepositoryConfig) rf, err := loadRepoConfig(m.RepositoryConfig)

@ -148,6 +148,12 @@ func TestGetRepoNames(t *testing.T) {
RepositoryConfig: repoConfig, RepositoryConfig: repoConfig,
RepositoryCache: repoCache, RepositoryCache: repoCache,
} }
// Helper function to generate expected key
expectedKey := func(name, repo string, index int) string {
return dependencyKey(name, repo, index)
}
tests := []struct { tests := []struct {
name string name string
req []*chart.Dependency req []*chart.Dependency
@ -159,7 +165,9 @@ func TestGetRepoNames(t *testing.T) {
req: []*chart.Dependency{ req: []*chart.Dependency{
{Name: "oedipus-rex", Repository: "http://example.com/test"}, {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", name: "no repo definition failure -- stable repo",
@ -173,28 +181,38 @@ func TestGetRepoNames(t *testing.T) {
req: []*chart.Dependency{ req: []*chart.Dependency{
{Name: "oedipus-rex", Repository: "http://example.com"}, {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", name: "repo from local path",
req: []*chart.Dependency{ req: []*chart.Dependency{
{Name: "local-dep", Repository: "file://./testdata/signtest"}, {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:)", name: "repo alias (alias:)",
req: []*chart.Dependency{ req: []*chart.Dependency{
{Name: "oedipus-rex", Repository: "alias:testing"}, {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 (@)", name: "repo alias (@)",
req: []*chart.Dependency{ req: []*chart.Dependency{
{Name: "oedipus-rex", Repository: "@testing"}, {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", name: "repo from local chart under charts path",
@ -203,13 +221,25 @@ func TestGetRepoNames(t *testing.T) {
}, },
expect: map[string]string{}, 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 { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
l, err := m.resolveRepoNames(tt.req) l, err := m.resolveRepoNames(tt.req)
if err != nil { if err != nil {
if tt.err { if tt.err {
continue return // Expected error, test passes
} }
t.Fatal(err) t.Fatal(err)
} }
@ -218,11 +248,67 @@ func TestGetRepoNames(t *testing.T) {
t.Fatalf("Expected error in test %q", tt.name) t.Fatalf("Expected error in test %q", tt.name)
} }
// m1 and m2 are the maps we want to compare // Compare maps
eq := reflect.DeepEqual(l, tt.expect) if !reflect.DeepEqual(l, tt.expect) {
if !eq { t.Errorf("%s: expected map %v, got %v", tt.name, tt.expect, l)
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)
} }
})
} }
} }

Loading…
Cancel
Save