Skip to content

[mirror] Honor --modules-path-suffix and warn on a moved modules path#410

Open
Glitchy-Sheep wants to merge 19 commits into
mainfrom
fix/mirror-fix-module-path-suffix
Open

[mirror] Honor --modules-path-suffix and warn on a moved modules path#410
Glitchy-Sheep wants to merge 19 commits into
mainfrom
fix/mirror-fix-module-path-suffix

Conversation

@Glitchy-Sheep

@Glitchy-Sheep Glitchy-Sheep commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

--modules-path-suffix was accepted by both d8 mirror push and d8 mirror pull but had no effect.

This PR makes it work on both sides, and warns in the pull/push summary when modules end up on a non-default path.

Problem

The flag was parsed into params on both commands and then ignored.

  • Push: modules always landed at <repo>/modules/<name>, whatever the flag value. The registry segment came from the unpacked bundle layout, and nothing read the flag.
  • Pull: the only reader was validateModulesAccess, which Execute never calls. Real discovery and image pulls hardcoded the modules segment.
  • Either way the command reported success with the wrong path, and never told the operator where modules actually went.

Fix

  • Push: rewrite the leading modules registry segment to the configured path, for both the layout push and the discovery-index tags.
  • Pull: make the modules path configurable in the registry service (WithModulesPathSuffix), and route discovery plus every module reference through one source of truth.
  • Both remaps live in registry space only. The bundle keeps its canonical modules/<name> layout on disk, so a mirror pulled with one suffix and pushed with another round-trips cleanly.
  • Empty value keeps the default modules; / places modules at the repo root.

Summary warning

  • The pull/push summary warns when modules use a non-default path (--modules-path-suffix).
  • It shows only the moved Modules row, in yellow, with a dimmed default: hint.
  • It fires only when the path was moved AND modules were actually transferred: at least one module pulled or planned on pull, at least one module repo pushed on push.
  • A default path, or a moved path no module went through, prints nothing.

Before / After

Before: --modules-path-suffix / is ignored. Push writes to <repo>/modules/<name>, pull reads from <repo>/modules/<name>.

2026-07-21 AT 19 26@2x

After: push writes to <repo>/<name>, pull reads from <repo>/<name>. Discovery index tags follow the modules to the same repo, and the summary warns about the moved path:

2026-07-23 AT 16 44@2x 2026-07-23 AT 16 47@2x

Tests

  • TestPushService_ModulesPathSuffix: runs PushService.Push on a fake registry for four suffix values; checks image path, discovery tag, an untouched non-module layout, and the summary (module count, moved flag).
  • TestService_ModulesPathSuffix: runs pull module discovery for the same four values; checks discovery and references agree on the source location.
  • TestBuildModulesPathReport: resolves the modules path and flags a moved path for default / / / custom / multi-segment values.
  • TestWriteModulesPathWarning: silent on a default path and on a moved-but-unused path; shows the moved row with a default: hint; emits colour only when enabled.
  • TestRenderPullSummary / TestRenderPushSummary: the warning fires only when the path was moved and modules were transferred.
  • Round-trip verified on a local registry: push a module with /, then pull it back with /.

Notes

  • Push and pull suffixes are independent. Pull reads from the source suffix; the bundle is always canonical modules/; push applies its own target suffix.
  • Help text for both commands was corrected: push describes a target-repo suffix, pull a source-repo suffix (it previously read as a source concept on push).
  • Shared summary rendering (colours, box helpers, the moved-path warning) moved to a new internal/mirror/summaryui package so pull and push look identical.
  • Small cleanups ride along: a shared PathToSegments helper replaces three copies of the same loop, a single NormalizeModulesPath keeps push and pull in sync, and a dead access-check that encoded the old suffix model is removed.
  • The "After" screenshot from the earlier revision is dropped: the summary no longer prints a full registry-layout table, so the block above is the current output. A fresh screenshot can replace the code block if preferred.

- `PushServiceOptions.ModulesPathSuffix` carries the flag value from `d8 mirror push` down to the push service.
- `modulesSegment()` resolves the registry segment for modules: empty option keeps the default `modules`, surrounding slashes are trimmed.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
@Glitchy-Sheep Glitchy-Sheep self-assigned this Jul 20, 2026
@Glitchy-Sheep Glitchy-Sheep added the bug Something isn't working label Jul 20, 2026
Rewrite the leading "modules" registry segment to the configured
suffix in both the layout push and the discovery-index step.

The bundle layout on disk is untouched, so the remap is registry-only
and legacy module-*.tar bundles keep working.

Empty suffix ("/") pushes modules to the target repo root.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- Cover the four `--modules-path-suffix` cases through `PushService.Push`: default and `/modules` keep `<repo>/modules/<name>`, `/` pushes to the repo root, `/my/mods` nests deeper.
- Each case checks the discovery index tag follows the modules to the same repo.
- A non-default suffix must leave nothing at the default `modules/` path, so modules are moved and not copied.
- A non-module `install` layout stays at `<repo>/install` for every suffix.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- The push flag and README described it as a source-repo suffix,
  which is a pull concept; on push it selects the target repo path.
- New text says what the flag does and how "/" pushes to the repo root.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
The modules registry segment was hardcoded to "modules" in the service
constructor. Add a WithModulesPathSuffix option so callers can point the
modules client elsewhere, defaulting to "modules" so existing behavior is
unchanged. Expose GetModulesSegment for building matching references.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Route every source-registry module reference through moduleRegistryPath,
which joins the configurable modules segment from the registry service.
Discovery-index scope and these references now share one source of truth,
so a non-default --modules-path-suffix points them at the same place. The
bundle-internal "modules" tar layout is left untouched.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Pass the pull flag into the registry service so module discovery and
image pulls read from the configured source path. The flag was parsed
but never reached discovery, which hardcoded the "modules" segment.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Mirror the push help wording on the read side and note that "/" reads
modules from the source repo root, so the round-trip flag is symmetric
and discoverable. Update the pull flag help and the README table.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
validateModulesAccess was never called by Execute and encoded the old
suffix semantics (suffix joined to the non-edition repo), which the fix
replaced. Delete it, its test, and the now-unused path import. Clarify
that the modules suffix is applied in NewService, not the RegistryPath
scoping above it.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
The scope-by-path loop was copied into push, the registry service, and a
test; suffix normalization was duplicated across push and the service.

Consolidate the split into pkgclient.PathToSegments (fed into the variadic
WithSegment) and export registryservice.NormalizeModulesPath, so push and
pull cannot drift on what a suffix means.

Rename the resolved modules location from "...Segment" to "...Path"
(modulesPath, GetModulesPath, NormalizeModulesPath): it holds a possibly
multi-segment path like "my/mods", not a single segment, and is distinct
from the raw ModulesPathSuffix flag value.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
@Glitchy-Sheep
Glitchy-Sheep marked this pull request as ready for review July 22, 2026 04:23
@Glitchy-Sheep
Glitchy-Sheep requested a review from ldmonster as a code owner July 22, 2026 04:23
Map each mirrored component to its registry path and flag a
non-default modules path (--modules-path-suffix). Shared by the
pull and push summaries.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Move the framed-summary colours, box helpers and formatters out of
the pull renderer into internal/mirror/summaryui so pull and push
share one look. Add WriteRegistryLayout for the registry layout
section, and document the rendered output with worked examples.
Pull aliases the moved primitives to stay unchanged.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Add a Registry section to the pull summary: the source repo and each
component's path, with a moved modules path (--modules-path-suffix)
highlighted. Shown under --verbose-summary or when the path differs
from the default.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Make PushService.Push return a PushSummary and collect what was
pushed (platform, installer, security DBs, module and package
counts). Render a framed push summary matching the pull one, always
showing the registry layout so a moved modules path
(--modules-path-suffix) is visible where it was applied.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Note in the pull and push README sections that the summary shows a
Registry layout and highlights a moved modules path.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
@Glitchy-Sheep Glitchy-Sheep changed the title [deckhouse-cli] Fix ignored --module-path-suffix argument [deckhouse-cli] Honor --modules-path-suffix and show it in the mirror summary Jul 23, 2026
Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- Print a `Warning:` header above the Registry block when `--modules-path-suffix` moves modules off the default.
- Warn in plain text, not by colour alone, so it holds up in piped logs and for colour-blind readers.
- Wrap the Registry block in blank lines so it reads apart from the component stats.
- Live in the shared `summaryui.WriteRegistryLayout`, so pull and push stay identical.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- The pull/push summary showed a full registry layout for every component; now it prints just the moved `Modules` row, in yellow with a `default:` hint.
- The warning fires only when `--modules-path-suffix` actually moved the path and modules went through it, so a default path or a module-free run stays silent.
- `--verbose-summary` no longer forces the section: paths shown only when there's something to act on.
- Replace the `RegistryLayout` model with a slim `ModulesPathReport` and rename `WriteRegistryLayout` to `WriteModulesPathWarning`.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
@Glitchy-Sheep Glitchy-Sheep changed the title [deckhouse-cli] Honor --modules-path-suffix and show it in the mirror summary [mirror] Honor --modules-path-suffix and warn on a moved modules path Jul 23, 2026
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