Update after feedback

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

@ -17,7 +17,7 @@ package resolver
import ( import (
"bytes" "bytes"
"crypto" "crypto/sha256"
"encoding/hex" "encoding/hex"
"encoding/json" "encoding/json"
"errors" "errors"
@ -54,13 +54,13 @@ func New(chartpath, cachepath string, registryClient *registry.Client) *Resolver
} }
} }
// dependencyKey creates a unique key for a dependency that includes name, repository, and index // 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 // to handle cases where multiple dependencies have the same name but different repositories
func dependencyKey(name, repository string, index int) string { func DependencyKey(name, repository string, index int) string {
// Use a combination of name, repository hash, and index to ensure uniqueness // Use a combination of name, repository hash, and index to ensure uniqueness
repoHash := "" repoHash := ""
if repository != "" { if repository != "" {
hasher := crypto.SHA256.New() hasher := sha256.New()
hasher.Write([]byte(repository)) hasher.Write([]byte(repository))
repoHash = hex.EncodeToString(hasher.Sum(nil))[:8] // Use first 8 chars for brevity repoHash = hex.EncodeToString(hasher.Sum(nil))[:8] // Use first 8 chars for brevity
} }
@ -123,9 +123,9 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
} }
// Try to get repository name using composite key first, then fall back to simple name // Try to get repository name using composite key first, then fall back to simple name
key := dependencyKey(d.Name, d.Repository, i) key := DependencyKey(d.Name, d.Repository, i)
repoName := repoNames[key] repoName := repoNames[key]
// Fall back to simple name lookup for backward compatibility // Fall back to simple name lookup for backward compatibility
if repoName == "" { if repoName == "" {
repoName = repoNames[d.Name] repoName = repoNames[d.Name]

@ -308,3 +308,59 @@ func TestGetLocalPath(t *testing.T) {
}) })
} }
} }
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)
}
})
}
}

@ -510,7 +510,7 @@ 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
depKey := dependencyKey(dd.Name, dd.Repository, i) depKey := resolver.DependencyKey(dd.Name, dd.Repository, i)
if _, ok := repoNames[depKey]; ok { if _, ok := repoNames[depKey]; ok {
continue continue
} }
@ -587,13 +587,13 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string,
fmt.Fprintf(m.Out, "Repository from local path: %s\n", dd.Repository) fmt.Fprintf(m.Out, "Repository from local path: %s\n", dd.Repository)
} }
// Use composite key for unique identification // Use composite key for unique identification
depKey := dependencyKey(dd.Name, dd.Repository, i) depKey := resolver.DependencyKey(dd.Name, dd.Repository, i)
reposMap[depKey] = dd.Repository reposMap[depKey] = dd.Repository
continue continue
} }
if registry.IsOCI(dd.Repository) { if registry.IsOCI(dd.Repository) {
depKey := dependencyKey(dd.Name, dd.Repository, i) depKey := resolver.DependencyKey(dd.Name, dd.Repository, i)
reposMap[depKey] = dd.Repository reposMap[depKey] = dd.Repository
continue continue
} }
@ -605,12 +605,12 @@ 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
depKey := dependencyKey(dd.Name, dd.Repository, i) depKey := resolver.DependencyKey(dd.Name, dd.Repository, i)
reposMap[depKey] = repo.Name 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
depKey := dependencyKey(dd.Name, dd.Repository, i) depKey := resolver.DependencyKey(dd.Name, dd.Repository, i)
reposMap[depKey] = repo.Name reposMap[depKey] = repo.Name
break break
} }
@ -620,7 +620,7 @@ 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 {
depKey := dependencyKey(dd.Name, repository, i) depKey := resolver.DependencyKey(dd.Name, repository, i)
reposMap[depKey] = repository reposMap[depKey] = repository
continue continue
} }
@ -648,19 +648,6 @@ 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)

@ -26,6 +26,7 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"helm.sh/helm/v4/internal/resolver"
chart "helm.sh/helm/v4/pkg/chart/v2" chart "helm.sh/helm/v4/pkg/chart/v2"
"helm.sh/helm/v4/pkg/chart/v2/loader" "helm.sh/helm/v4/pkg/chart/v2/loader"
chartutil "helm.sh/helm/v4/pkg/chart/v2/util" chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
@ -151,7 +152,7 @@ func TestGetRepoNames(t *testing.T) {
// Helper function to generate expected key // Helper function to generate expected key
expectedKey := func(name, repo string, index int) string { expectedKey := func(name, repo string, index int) string {
return dependencyKey(name, repo, index) return resolver.DependencyKey(name, repo, index)
} }
tests := []struct { tests := []struct {
@ -256,62 +257,6 @@ func TestGetRepoNames(t *testing.T) {
} }
} }
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)
}
})
}
}
func TestDownloadAll(t *testing.T) { func TestDownloadAll(t *testing.T) {
chartPath := t.TempDir() chartPath := t.TempDir()
m := &Manager{ m := &Manager{

Loading…
Cancel
Save