Skip to content

[deckhouse-cli] Fix --include-module ignoring semver constraints#341

Open
Glitchy-Sheep wants to merge 14 commits intomainfrom
fix/include-module-flag-ignored-during-pull
Open

[deckhouse-cli] Fix --include-module ignoring semver constraints#341
Glitchy-Sheep wants to merge 14 commits intomainfrom
fix/include-module-flag-ignored-during-pull

Conversation

@Glitchy-Sheep
Copy link
Copy Markdown
Contributor

@Glitchy-Sheep Glitchy-Sheep commented Apr 29, 2026

[mirror] Fix --include-module ignoring semver constraints during pull

Summary

d8 mirror pull --include-module <name>@<semver> was pulling only the channel version, not the full semver range.

The fix gives the version filter the actual tag list from the registry, so the constraint matches every fitting version.

Problem

  • pullSingleModule built &Module{Releases: nil} before calling Filter.VersionsToMirror. The semver branch of the filter iterates Releases, so it had nothing to match.
  • Bundle ended up with only v1.45.2 (the channel version), missing v1.40.0 through v1.44.0 that all matched ^1.40.0 in the registry.
  • Exact-tag constraints (@=v1.40.0) were not affected - they don't read Releases.
  • Unit tests for the filter populated Releases by hand, so they didn't catch the integration gap.

Fix

  • pullSingleModule now calls modulesService.Module(name).ListTags(ctx) and stores the result on Module.Releases. The semver branch sees every published version.
  • The call is guarded by hasConstraint && !constraint.IsExact(), so it is skipped for exact-tag whitelist and for blacklist-mode modules that don't have a constraint. A default pull (no --include-module) makes zero extra requests across the registry.

Behavior Map:

Registry tags: v1.39.0, v1.40.0, v1.40.1, v1.41.0, v1.42.0, v1.43.0, v1.45.2. Channels → v1.45.2.

Legend: ✅ correct, ❌ broken

# Constraint Expected Before After
1 console (no @) all (>=0.0.0) only v1.45.2 all 7 ✅
2 @1.40.0 (implicit ^) >=1.40.0 <2.0.0 only v1.45.2 6 matching ✅
3 @^1.40.0 >=1.40.0 <2.0.0 only v1.45.2 6 matching ✅
4 @~1.40.0 >=1.40.0 <1.41.0 only v1.45.2 v1.40.0, v1.40.1 + channel ✅
5 @>=1.40.0 <1.43.0 range only v1.45.2 4 matching + channel ✅
6 @>1.42.0 >1.42.0 only v1.45.2 v1.43.0, v1.45.2
7 @<1.41.0 <1.41.0 wrong v1.45.2 v1.39.0, v1.40.0, v1.40.1 + channel ✅
8 @=v1.40.0 exact, no channel pull, alias under all 5 channels v1.40.0 + 5 channel aliases ✅ v1.40.0 + 5 channel aliases ✅ unchanged
9 @=v1.40.0+stable exact, alias under stable only v1.40.0 + stable alias ✅ v1.40.0 + stable alias ✅ unchanged

Notes:

  • Row 1: bare --include-module foo maps to >=0.0.0 internally → all historical versions of the module. Before the fix this was masked (only channel snapshot pulled). After the fix, real registries with ~100+ historical tags produce dramatically larger bundles.

⚠️ Other changes in this PR:

  • --include-module and --exclude-module are now mutually exclusive at the cobra flag level. Before, both could be passed and whitelist silently won. The flags logic was simplified.
  • Renamed c / ok / e / sc in Filter.VersionsToMirror and Filter.ShouldMirrorReleaseChannels to constraint / hasConstraint / exact / isExactTag / semver / isSemver for clarity.
  • Cleaned up doc comments around the modules pull pipeline.
  • Refactored: internal/mirror/modules/modules.go - divided monolithic hard to read function into modular functions.

Before / After

Showcase: --include-module console@1.40.0 (semver ^1.40.0) - the canonical case from the bug report. Other syntax variants (console, console@~1.40.0, console@=v1.40.0) were also verified manually with no regression.

d8 mirror pull --source registry.deckhouse.ru/deckhouse/ee --license <token> \
  --no-platform --no-security-db --dry-run \
  --include-module console@1.40.0 /tmp/d8-mirror-test

Before: only the channel snapshot (v1.45.2). The ^1.40.0 constraint had no effect.
2026-04-29 AT 13 17

After: every tag in the registry that matches ^1.40.0 - 33 versions from v1.40.0 through v1.45.2.
2026-04-29 AT 13 18

Test plan

Covered in internal/mirror/modules/pull_versions_test.go: semver matching (caret / tilde / range), multi-module independence, the per-module ListTags skip optimization, and both error-paths (ErrImageNotFound skipped vs other errors propagated).

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
It's obvious from the flags logic itself, and it simplifies the following code.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
… zero value (nil) + test for the behavior + names refactor

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- `ListTags` for a module's tag list now runs only when the constraint is semver. Exact-tag and blacklist-mode modules take filter branches that never read `Module.Releases`, so the call was wasted work.
- For a default `d8 mirror pull` (no `--include-module`), per-module `ListTags` drops to zero across the registry.
- Adds tests verifying the skip for exact-tag and blacklist cases, plus a counter-check that semver constraint still triggers the call.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
@Glitchy-Sheep Glitchy-Sheep self-assigned this Apr 29, 2026
@Glitchy-Sheep Glitchy-Sheep added the bug Something isn't working label Apr 29, 2026
Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
…ration)

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- `Module(name).ListTags` now warns and falls back to channel-only versions on `client.ErrImageNotFound`, matching the policy of `validateModulesAccess`. Other errors keep the existing fail-fast behavior.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- 8 new phase methods make the orchestrator read top-to-bottom as a plan.
- Each phase preserves its error policy: fail-fast for required pulls, debug-log-and-continue for release-version and internal-digest pulls where missing artifacts are expected.
- All 10 phases live right after the orchestrator (`pullVexImages` and `findVexImage` moved into the cluster), so the call order is visible in file order.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- `TestPullModules_*` names match the public entry point each test exercises.
- The per-module `ListTags` policy is covered by two table-driven tests: call count and error handling.
- Assertions read the download list through the public `Module(name)` accessor.
- File is split into named sections - fixtures, builders, assertions, test doubles - with consistent helper names.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
@Glitchy-Sheep Glitchy-Sheep marked this pull request as ready for review April 30, 2026 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant