helm: support versioning-strategy (range-preserving updates)#15218
helm: support versioning-strategy (range-preserving updates)#15218casey-robertson-paypal wants to merge 24 commits into
Conversation
Adapts NpmAndYarn::Requirement to parse Helm Chart.yaml constraints (caret, tilde, hyphen, x-range, OR, space- and comma-separated AND). parse always returns a Helm::Version so satisfaction comparisons stay Helm::Version vs Helm::Version (its <=> sig rejects plain Gem::Version). Not registered globally; used explicitly by the range-preserving requirements updater. No behavior change (no caller yet).
Mirrors npm's RequirementsUpdater for increase (BumpVersions), increase-if-necessary (BumpVersionsIfNecessary), and widen (WidenRanges), scoped to Helm and built on Helm::Requirement/Helm::Version. Widen follows npm semantics (caret bump, <-bound widened in place), not Renovate's OR-append. Not wired into the live update checker yet; no behavior change.
Route the Chart.yaml constraint (stored in dependency.version for helm chart deps) through RequirementsUpdater so increase-if-necessary and widen are honored, suppress in-range updates under range-preserving strategies (no PR when the resolved version already satisfies the constraint), and write the updated requirement string into Chart.yaml. The default (increase / BumpVersions) reproduces the previous exact-pin behavior, so existing users see no change unless they opt into a non-default strategy.
Anchor candidate filtering on the lowest version named in the constraint so explicit ranges (>=1.0.0 <2.0.0, comma, ||) no longer crash version_class.new, and quote rewritten requirements that need it so Chart.yaml stays valid YAML. Adds strategy tests for tilde, explicit range, comma-AND, and OR.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds range-preserving update behavior for Helm Chart.yaml dependencies by introducing Helm-specific requirement parsing and a requirements updater that honors Dependabot’s versioning strategies.
Changes:
- Add
Dependabot::Helm::Requirementto parse Masterminds-style SemVer ranges (caret, tilde, comparator ranges, OR, etc.). - Add
Dependabot::Helm::UpdateChecker::RequirementsUpdaterand wire it into the Helm update checker for:helm_chartdependencies. - Update the Helm chart file updater to write updated requirement strings (and quote them when needed for valid YAML), plus add fixtures and specs covering range behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| helm/spec/fixtures/helm/charts/chart_with_range_dependency.yaml | Adds fixture for caret-range dependencies in Chart.yaml. |
| helm/spec/fixtures/helm/charts/chart_with_explicit_range.yaml | Adds fixture for explicit comparator-range dependencies that require quoting. |
| helm/spec/dependabot/helm/update_checker_spec.rb | Extends update checker specs to cover range-preserving strategies and comparator ranges. |
| helm/spec/dependabot/helm/update_checker/requirements_updater_spec.rb | Adds focused unit tests for requirements rewriting under multiple strategies and range syntaxes. |
| helm/spec/dependabot/helm/requirement_spec.rb | Adds tests for Helm requirement parsing/satisfaction across supported range types. |
| helm/spec/dependabot/helm/file_updater/chart_updater_spec.rb | Ensures Chart.yaml output uses updated requirement strings and quotes comparator ranges to keep YAML valid. |
| helm/lib/dependabot/helm/update_checker/requirements_updater.rb | Introduces Helm-specific requirements updater (mirroring npm-like semantics). |
| helm/lib/dependabot/helm/update_checker.rb | Integrates requirements updater, adds range-aware can_update? behavior and safer current-version anchoring. |
| helm/lib/dependabot/helm/requirement.rb | Implements Helm/Masterminds-style requirement parsing and conversion to Ruby constraints. |
| helm/lib/dependabot/helm/file_updater/chart_updater.rb | Writes updated requirement strings back to Chart.yaml and quotes unsafe YAML scalars. |
| def self.requirements_array(requirement_string) | ||
| return [new(nil)] if requirement_string.nil? | ||
|
|
||
| # Parentheses are extremely rare in Helm constraints; strip them. | ||
| requirement_string = requirement_string.gsub(/[()]/, "") | ||
| requirement_string.strip.split(OR_SEPARATOR).map do |req_string| | ||
| new(req_string.strip.split(AND_SEPARATOR)) | ||
| end | ||
| end | ||
|
|
||
| sig { params(requirements: T.nilable(T.any(String, T::Array[String]))).void } | ||
| def initialize(*requirements) | ||
| requirements = requirements.flatten | ||
| .flat_map { |req_string| T.must(req_string).split(",").map(&:strip) } | ||
| .flat_map { |req_string| convert_helm_constraint_to_ruby_constraint(req_string) } | ||
|
|
||
| super | ||
| end |
There was a problem hiding this comment.
Fixed in be6cf17. new(nil) (from requirements_array(nil)) now resolves to a >= 0 match-anything constraint backed by a Helm::Version, instead of raising on T.must(nil). Added a spec covering the nil case.
| if current_requirement.match?(/(<|-\s)/i) | ||
| ruby_req = ruby_requirements(current_requirement).first | ||
| return req if ruby_req&.satisfied_by?(latest_resolvable_version) | ||
|
|
||
| updated_req = update_range_requirement(current_requirement) | ||
| return req.merge(requirement: updated_req) | ||
| end |
There was a problem hiding this comment.
Fixed in be6cf17. The range branch now checks any? across all OR alternatives instead of only .first, so a later alternative that already permits the latest version is honored. Added a spec covering <1.0.0 || >=2.0.0.
| named = dependency.version.to_s.scan(/\d+(?:\.\d+)+/).filter_map do |v| | ||
| version_class.new(v) if version_class.correct?(v) | ||
| end | ||
| named.min || version_class.new("0") |
There was a problem hiding this comment.
Adjusted in be6cf17. The anchor scan is now \d+(?:\.\d+)*, so major-only tokens (e.g. >=1 <2) anchor on 1 rather than falling back to 0. (This fallback only runs for multi-comparator ranges that can't parse as a single version; for those the anchor just sets the floor for candidate filtering, so the resolved latest is unaffected — but matching major-only tokens is more correct.)
…or regex) - Requirement#initialize: nil requirement now means match-anything (>= 0) instead of raising on T.must(nil). - RequirementsUpdater: check all OR alternatives, not just the first, when deciding whether the latest version is already permitted. - UpdateChecker#current_version: anchor regex also matches major-only tokens.
- Relax parse / latest_resolvable_version sigs to Dependabot::Version (the statically-inferred type of version_class.new). - T.must the RequirementsUpdater result (.first is nilable). - T.cast scan results to String before correct?. - Resolve nilable Helm::Version#to_s in update_version_string.
|
Re: the red Smoke check — it's missing helm smoke wiring, not this PR's code.
I'll wire it up — a |
Use possessive quantifiers in VERSION_REGEX, SEPARATOR, and the range matcher so they stay linear on pathological input (resolves the CodeQL polynomial-regex alerts). Matching is identical to the greedy forms for real version constraints; full spec suite unchanged.
Open decision: default behavior for caret/range constraintsThere's a conflict the issue's "
These coincide for exact pins ( Prior art — all of it points toward preserving ranges:
So operator-preserving reads as the correct, ecosystem-consistent default, and helm's exact-pin looks like a pre- That said, it's a backward-compat decision for your users, so I'd rather you make the call:
Happy to implement either shape. Path forward: point me at a default, I'll finalize the code if it needs adjusting, then land the helm smoke-tests companion (its recorded expectations depend on this decision). The docs PR (github/docs#44584) is already open. |
BumpVersions preserves the authored operator (^1.0.0 -> ^1.0.5); it does not reproduce helm's prior exact-pin behavior. See the default-behavior discussion on the PR.
Resolve conflict in helm/update_checker.rb: combine main's DependencyRequirement return type + wrap_requirements helper with the versioning-strategy routing (helm_chart deps through RequirementsUpdater, image deps exact-overwrite).
Matches the existing .rubocop_todo.yml treatment of every other ecosystem's requirements_updater (all use T::Hash[Symbol, T.untyped] for the requirement hash, all grandfathered under this cop).
|
Hey @thavaahariharangit! Hoping I could get your eyes on this when you have some spare cycles (or a pointer to whoever owns helm these days). It's been open about two weeks, CI and CodeQL are green, and it's caught up with main. There's one thing I'd really value a maintainer's read on before I finalize. Should helm's default (when no No rush at all, and totally fine if you want to hand it off. Thanks! |
Implements #15216 — wires
versioning-strategyinto thehelmecosystem soChart.yamlconstraints can be preserved instead of always exact-pinned.Behavior
increaseincrease-if-necessarywiden^1.0.0^1.0.5^1.0.0^2.0.0^2.0.0^2.0.0~1.2.0~1.2.9>=1.0.0 <2.0.0>=1.0.0 <3.0.0>=1.0.0 <3.0.0>=1.0.0 <3.0.01.0.0(exact)1.5.01.5.01.5.0How it works
Helm::Requirement— a SemVer-range parser (helm uses Masterminds-style^/~/ranges/||, unlike the gem-styleDocker::Requirementregistered today). Modeled onNpmAndYarn::Requirement; handles space- and comma-separated AND.RequirementsUpdater— mirrors npm's, scoped to the three strategies.UpdateChecker/ChartUpdaterwiring — in-range updates are suppressed (no PR) under range-preserving strategies; multi-comparator ranges are anchored on their lowest named version (so they don't crash the version check) and rewritten requirements are quoted to keepChart.yamlvalid YAML.A few design choices I'd genuinely welcome input on
increasesemantics — see the open-question comment below (legacy exact-pin vs. documented operator-preservingincrease).Helm::Requirementis used explicitly in the new code paths rather than re-registered globally, to avoid disturbing the docker-image path / ignore conditions. Happy to switch to a single registered class if preferred.widenmirrors npm's exact semantics (caret/tilde bumped in place;</hyphen bounds widened; OR-append only for un-widenable ranges), not Renovate's||style — chosen for consistency with other Dependabot ecosystems.Testing
Unit specs for each piece (parser, updater, checker, chart updater) covering caret / tilde / exact / explicit-range / comma / OR across all three strategies.
Plus an end-to-end verification repo: casey-robertson-paypal/dependabot-helm runs this branch via
dry-runagainst real OCI charts for all five constraint styles — committed transcripts + green CI + a results table in the run summary.Companion docs PR: github/docs#44584 (adds
helmto theversioning-strategyreference). A helm smoke-tests companion is also planned once the default-behavior question below is settled.Putting this out as a functional, tested starting point — I'm not attached to any specific choice here and would love feedback or a different direction. Just excited to help close the gap.