helm: support versioning-strategy (range-preserving updates)#15218
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. |
…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).
… public consumer
Follow-up review fixes:
- min_bound_anchor: an OR constraint with a branch that has no lower bound
("<=2.0.0 || >=10.0.0") permits arbitrarily low versions, so anchor at 0
rather than the other branch's floor.
- Drop the direct current_version private-method assertions; verify the anchor
through filter_valid_releases (the public-facing consumer the spec already
covers), matching the file's existing convention.
update_range_requirement replaced the first textual occurrence of the upper bound, which could hit an earlier lower-bound operand sharing the same version string. Anchor the substitution to the < / <= / hyphen so only the upper bound moves.
Follow-up to the OR-anchor fix: min_version reports nil for an exact (=) requirement, so "1.0.0 || 2.0.0" was treated as unbounded-below and anchored at 0. An exact branch's floor is its pinned version; only genuinely lower-unbounded branches (< / <= / !=) fall back to 0.
The multi-file skip-unchanged guard also masked genuine rewrite failures: an entry the regex can't locate (e.g. a quoted name, or version listed before name) produced unchanged content that the file updater then skipped, emitting a partial PR with no error. Now the writer skips only entries whose constraint is genuinely unchanged, tolerates a quoted name, and raises when a *changed* constraint has no matching entry.
current_version anchored on the single combined dependency.version, so when a chart is merged from several occurrences with different constraints, a higher- floored occurrence (e.g. ^5.0.0) could gate out a release that a lower one (^1.0.0) still needs updated to. Anchor on the lowest occurrence constraint instead, consistent with the per-occurrence handling elsewhere.
| # This occurrence's constraint is unchanged (the strategy left it | ||
| # alone) — nothing to write for it. | ||
| next if new_requirement == old_version | ||
|
|
||
| new_content, cursor = replace_next_entry_version( |
| end | ||
| def replace_next_entry_version(content, cursor, old_version, new_version) | ||
| pattern = / | ||
| (\s+-\s+name:\s+["']?#{Regexp.escape(dependency.name)}["']?.*?\n\s+version:\s+) |
| def updated_chart_requirement(req) | ||
| current_constraint = chart_constraint_for(req) | ||
| synthetic = T.cast(req.merge(requirement: current_constraint), Dependabot::DependencyRequirement) | ||
|
|
||
| T.must( | ||
| RequirementsUpdater.new( | ||
| requirements: [synthetic], | ||
| update_strategy: resolved_update_strategy, | ||
| latest_resolvable_version: T.must(latest_version).to_s | ||
| ).updated_requirements.first | ||
| ) | ||
| end |
| end | ||
| def replace_next_entry_version(content, cursor, old_version, new_version) | ||
| pattern = / | ||
| (\s+-\s+name:\s+["']?#{Regexp.escape(dependency.name)}["']?.*?\n\s+version:\s+) |
|
@casey-robertson-paypal Thanks for the contribution, this change has now been deployed! Really appreciate you adding the version strategy for Helm |
Implements #15216 — wires
versioning-strategyinto thehelmecosystem soChart.yamlconstraints can be preserved instead of always exact-pinned.🔖 Open decisions for reviewers
Status: All review feedback has been triaged and addressed. Correctness has been verified two ways: a full unit suite, and a differential test that runs 735 constraint×version pairs through both this parser and the real Go Masterminds/semver library helm uses — 0 divergences on every common form (caret incl. 0.x, tilde/
~>, comparators,!=, comma/space-AND, hyphen incl. partial, OR, wildcards, comparator+wildcard, v-prefix,+digest). A holistic review pass also fixed several structural bugs (file-corruption and a crash in multi-occurrence handling, segment/empty-constraint crashes) and simplified the constraint anchor to useRequirement#min_version. CI is green. Three decisions remain — all genuinely a maintainer's call, not oversights:Default
increasesemantics — for caret/range users who have not setversioning-strategy, this PR currently applies the documented operator-preserving behavior (^1.0.0→^1.0.5) instead of helm's historical exact-pin (^1.0.0→1.0.5). Exact pins are unaffected (1.0.0→1.5.0). Prior art favors operator-preserving (Helm's own chart best-practices, Renovate, and every other Dependabot ecosystem), but changing the default for un-opted users is a backward-compat decision I don't want to make unilaterally — happy to make it fully opt-in instead. → discussionFull Masterminds fidelity for exotic constraint forms — the common forms match Masterminds exactly (the 0/735 differential above). A handful of rare forms still follow RubyGems rather than Masterminds semantics, because
Helm::Requirementconverts toGem::Requirement: prereleases admitted within ordinary ranges (^1.0.0vs1.1.0-rc1),=/!=x-ranges (=1.x,!=1.x), partial comparators (>1.2starts at 1.3.0), and~0.0.0. These are uncommon in realChart.yamls and closing them fully touches core satisfaction, so I've proposed them as a focused follow-up rather than expanding this PR — but happy to land them here if you'd prefer.Scope: keep or drop multi-occurrence handling? When the same chart name is depended on from multiple files/entries with different constraints,
DependencySetmerges them into one dependency with several requirements, and this PR updates each occurrence by its ownsource[:tag]. It works and is fully tested now, but honestly it's been the source of most of the review churn — the per-occurrence machinery is combinatorial and generated several edge-case bugs (file-corruption on aliasing entries, an "unchanged file" crash, and an anchor that gated on the wrong occurrence — all fixed + regression-tested). It supports a case that's rare in real charts. Option A: keep it as-is (hardened). Option B: drop it to a focused follow-up and handle only the common one-requirement-per-chart case here, shrinking the PR. I lean slightly toward B for reviewability, but it's a scope call I'd rather you make.Everything else raised in review is resolved.
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,+build/digest suffixes, hyphen ranges, and x-ranges.RequirementsUpdater— mirrors npm's, scoped to the three strategies.UpdateChecker/ChartUpdaterwiring — in-range updates are suppressed (no PR) under range-preserving strategies;version_can_update?is gated on whether the authored constraint would actually change (so it never hands the file updater an unchangedChart.yaml); multi-comparator ranges are anchored on their lowest lower-bound version; each requirement is updated by its ownsource[:tag](a chart depended on from multiple files keeps its per-occurrence constraint); and rewritten requirements are quoted to keepChart.yamlvalid YAML.A few design choices I'd genuinely welcome input on
increasesemantics — see decision (1) above.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 / hyphen / comma / OR / wildcard / build-metadata across all three strategies, plus a round-trip invariant guard (any
Helm::Version-valid pin parses and self-satisfies).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 above 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.