From bb66fb3365aa83dcbace9ea1ea3a105a1a156c6b Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Fri, 3 Sep 2021 19:23:35 -0300 Subject: [PATCH 1/5] Make `version` optional for local chart dependencies Fixes #10095 Signed-off-by: Felipe Santos --- cmd/helm/dependency.go | 2 +- internal/resolver/resolver.go | 51 ++++++++++++++++++------------ internal/resolver/resolver_test.go | 22 +++++++++++++ 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go index 03874742c..7a41a0b78 100644 --- a/cmd/helm/dependency.go +++ b/cmd/helm/dependency.go @@ -70,7 +70,7 @@ the dependency charts stored locally. The path should start with a prefix of If the dependency chart is retrieved locally, it is not required to have the repository added to helm by "helm add repo". Version matching is also supported -for this case. +for this case, but optional. ` const dependencyListDesc = ` diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 5e8921f96..1e5ca0407 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -58,31 +58,36 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string locked := make([]*chart.Dependency, len(reqs)) missing := []string{} for i, d := range reqs { - constraint, err := semver.NewConstraint(d.Version) - if err != nil { - return nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name) - } + var constraint *semver.Constraints + var chartpath string + var err error - if d.Repository == "" { - // Local chart subfolder - if _, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath); err != nil { - return nil, err - } - - locked[i] = &chart.Dependency{ - Name: d.Name, - Repository: "", - Version: d.Version, + // If version is defined + if d.Version != "" { + constraint, err = semver.NewConstraint(d.Version) + if err != nil { + return nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name) } - continue } - if strings.HasPrefix(d.Repository, "file://") { - chartpath, err := GetLocalPath(d.Repository, r.chartpath) - if err != nil { - return nil, err + // Local chart + if d.Repository == "" || strings.HasPrefix(d.Repository, "file://") { + + if d.Repository == "" { + // From charts subfolder + chartpath, err = GetLocalPath(filepath.Join("charts", d.Name), r.chartpath) + if err != nil { + return nil, err + } + } else { + // From file:// repository + chartpath, err = GetLocalPath(d.Repository, r.chartpath) + if err != nil { + return nil, err + } } + // Load chart to validate the version ch, err := loader.LoadDir(chartpath) if err != nil { return nil, err @@ -90,11 +95,15 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string v, err := semver.NewVersion(ch.Metadata.Version) if err != nil { - // Not a legit entry. + // If version is not set and the version from the local chart is invalid + if d.Version == "" { + missing = append(missing, d.Name) + } continue } - if !constraint.Check(v) { + // If version is set but does not match the local chart + if d.Version != "" && !constraint.Check(v) { missing = append(missing, d.Name) continue } diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index a79852175..f12045155 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -137,6 +137,28 @@ func TestResolve(t *testing.T) { }, err: true, }, + { + name: "no version set for local chart under charts path", + req: []*chart.Dependency{ + {Name: "localdependency", Repository: "", Version: ""}, + }, + expect: &chart.Lock{ + Dependencies: []*chart.Dependency{ + {Name: "localdependency", Repository: "", Version: "0.1.0"}, + }, + }, + }, + { + name: "no version set for local chart from file://", + req: []*chart.Dependency{ + {Name: "localdependency", Repository: "file://base", Version: ""}, + }, + expect: &chart.Lock{ + Dependencies: []*chart.Dependency{ + {Name: "localdependency", Repository: "file://base", Version: "0.1.0"}, + }, + }, + }, } repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"} From 34e9c7ad57307baea71cadc5a7525fd9de6e18f5 Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Sun, 23 Feb 2025 22:47:46 -0300 Subject: [PATCH 2/5] Fix some simple comments Signed-off-by: Felipe Santos --- cmd/helm/dependency.go | 4 ++-- internal/resolver/resolver.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go index 99b9b5a05..df2bd9427 100644 --- a/cmd/helm/dependency.go +++ b/cmd/helm/dependency.go @@ -70,8 +70,8 @@ the dependency charts stored locally. The path should start with a prefix of repository: "file://../dependency_chart/nginx" If the dependency chart is retrieved locally, it is not required to have the -repository added to helm by "helm add repo". Version matching is also supported -for this case, but optional. +repository added to helm by "helm add repo". Version matching is optionally +supported for this case. ` const dependencyListDesc = ` diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index c95e0703a..e695a179c 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -94,7 +94,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string v, err := semver.NewVersion(ch.Metadata.Version) if err != nil { - // If version is not set and the version from the local chart is invalid if d.Version == "" { missing = append(missing, d.Name) } From a6750a0d87f3ad471ac50a367f298b32673c45d8 Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Wed, 9 Jul 2025 18:17:50 -0300 Subject: [PATCH 3/5] Address a very minor comment Signed-off-by: Felipe Santos --- internal/resolver/resolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index fb04aa366..b0f445677 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -96,7 +96,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string v, err := semver.NewVersion(ch.Metadata.Version) if err != nil { if d.Version == "" { - missing = append(missing, d.Name) + missing = append(missing, fmt.Sprintf("%q (repository %q)", d.Name, d.Repository)) } continue } From 7e68503632d5f997516978a1e8b6123d51dd8e63 Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Wed, 9 Jul 2025 18:39:57 -0300 Subject: [PATCH 4/5] Address other comments through Claude Sonnet 4 Signed-off-by: Felipe Santos --- internal/resolver/resolver.go | 75 ++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index b0f445677..b3269d8da 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -59,35 +59,44 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string locked := make([]*chart.Dependency, len(reqs)) missing := []string{} for i, d := range reqs { - var constraint *semver.Constraints - var chartpath string - var err error - - // If version is defined - if d.Version != "" { - constraint, err = semver.NewConstraint(d.Version) - if err != nil { - return nil, fmt.Errorf("dependency %q has an invalid version/constraint format: %w", d.Name, err) + // Handle local chart dependencies (empty repository) + if d.Repository == "" { + // Local chart subfolder - maintain backward compatibility + if _, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath); err != nil { + return nil, err } - } - // Local chart - if d.Repository == "" || strings.HasPrefix(d.Repository, "file://") { - if d.Repository == "" { - // From charts subfolder - chartpath, err = GetLocalPath(filepath.Join("charts", d.Name), r.chartpath) + // For empty repository, determine version from local chart if not specified + version := d.Version + if version == "" { + chartpath, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath) if err != nil { return nil, err } - } else { - // From file:// repository - chartpath, err = GetLocalPath(d.Repository, r.chartpath) + + ch, err := loader.LoadDir(chartpath) if err != nil { return nil, err } + + version = ch.Metadata.Version + } + + locked[i] = &chart.Dependency{ + Name: d.Name, + Repository: "", + Version: version, + } + continue + } + + // Handle file:// repository dependencies + if strings.HasPrefix(d.Repository, "file://") { + chartpath, err := GetLocalPath(d.Repository, r.chartpath) + if err != nil { + return nil, err } - // Load chart to validate the version ch, err := loader.LoadDir(chartpath) if err != nil { return nil, err @@ -95,16 +104,24 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string v, err := semver.NewVersion(ch.Metadata.Version) if err != nil { + // If no version specified, add to missing list if d.Version == "" { missing = append(missing, fmt.Sprintf("%q (repository %q)", d.Name, d.Repository)) } continue } - // If version is set but does not match the local chart - if d.Version != "" && !constraint.Check(v) { - missing = append(missing, fmt.Sprintf("%q (repository %q, version %q)", d.Name, d.Repository, d.Version)) - continue + // If version is specified, validate it against the local chart + if d.Version != "" { + constraint, err := semver.NewConstraint(d.Version) + if err != nil { + return nil, fmt.Errorf("dependency %q has an invalid version/constraint format: %w", d.Name, err) + } + + if !constraint.Check(v) { + missing = append(missing, fmt.Sprintf("%q (repository %q, version %q)", d.Name, d.Repository, d.Version)) + continue + } } locked[i] = &chart.Dependency{ @@ -115,6 +132,16 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string continue } + // Handle remote repository dependencies + var constraint *semver.Constraints + if d.Version != "" { + var err error + constraint, err = semver.NewConstraint(d.Version) + if err != nil { + return nil, fmt.Errorf("dependency %q has an invalid version/constraint format: %w", d.Name, err) + } + } + repoName := repoNames[d.Name] // if the repository was not defined, but the dependency defines a repository url, bypass the cache if repoName == "" && d.Repository != "" { @@ -189,7 +216,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string // Not a legit entry. continue } - if constraint.Check(v) { + if constraint == nil || constraint.Check(v) { found = true locked[i].Version = v.Original() break From 8b4cbb02988108455c748094ef5c81c875a31a35 Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Wed, 9 Jul 2025 18:59:39 -0300 Subject: [PATCH 5/5] Rewrite after Claude Sonnet 4 changes Signed-off-by: Felipe Santos --- internal/resolver/resolver.go | 40 ++++++++++++++--------------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index b3269d8da..706357a89 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -59,21 +59,28 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string locked := make([]*chart.Dependency, len(reqs)) missing := []string{} for i, d := range reqs { + var constraint *semver.Constraints + + // Validate version early if set, regardless of local or remote + if d.Version != "" { + var err error + constraint, err = semver.NewConstraint(d.Version) + if err != nil { + return nil, fmt.Errorf("dependency %q has an invalid version/constraint format: %w", d.Name, err) + } + } + // Handle local chart dependencies (empty repository) if d.Repository == "" { - // Local chart subfolder - maintain backward compatibility - if _, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath); err != nil { + chartpath, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath) + if err != nil { return nil, err } - // For empty repository, determine version from local chart if not specified version := d.Version - if version == "" { - chartpath, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath) - if err != nil { - return nil, err - } + // Determine version from local chart if not specified + if version == "" { ch, err := loader.LoadDir(chartpath) if err != nil { return nil, err @@ -113,11 +120,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string // If version is specified, validate it against the local chart if d.Version != "" { - constraint, err := semver.NewConstraint(d.Version) - if err != nil { - return nil, fmt.Errorf("dependency %q has an invalid version/constraint format: %w", d.Name, err) - } - if !constraint.Check(v) { missing = append(missing, fmt.Sprintf("%q (repository %q, version %q)", d.Name, d.Repository, d.Version)) continue @@ -132,16 +134,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string continue } - // Handle remote repository dependencies - var constraint *semver.Constraints - if d.Version != "" { - var err error - constraint, err = semver.NewConstraint(d.Version) - if err != nil { - return nil, fmt.Errorf("dependency %q has an invalid version/constraint format: %w", d.Name, err) - } - } - repoName := repoNames[d.Name] // if the repository was not defined, but the dependency defines a repository url, bypass the cache if repoName == "" && d.Repository != "" { @@ -216,7 +208,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string // Not a legit entry. continue } - if constraint == nil || constraint.Check(v) { + if constraint.Check(v) { found = true locked[i].Version = v.Original() break