Skip to content

[deckhouse-cli] New plugins contract support - refactor [1/2]#346

Merged
ldmonster merged 5 commits into
mainfrom
feat/new-plugins-contract-stage1-refactor
May 13, 2026
Merged

[deckhouse-cli] New plugins contract support - refactor [1/2]#346
ldmonster merged 5 commits into
mainfrom
feat/new-plugins-contract-stage1-refactor

Conversation

@Glitchy-Sheep
Copy link
Copy Markdown
Contributor

@Glitchy-Sheep Glitchy-Sheep commented May 12, 2026

Summary

This pr is pure refactor before new contract support:

  • No behaviour change
  • no CLI surface change (only deadcode removed)

The implementation stage (PR [2/2]) is stacked on top of this branch and ships separately.

⭐ The problem refactor solves

cmd/plugins/plugins.go - a ~1000-line file mixing flag parsing, registry I/O, validation, filesystem layout and CLI output.

There are several problems with it:

  • It violates design principles of others cli commands (inconsistency of layout)
  • Not really readable (mixed logic in 1000 lines of code)
  • Magic values in path resolution for placing plugins files (1+ source of truth), it's hard to manage

🚶‍♂️‍➡️ Refactor walkthrough

  1. Split the monolithic file.

    • The 1000-line plugins.go was broken into one file per concern - install, update, remove, list, contract, validators.
    • The logic was moved to internal/plugins/cmd/, matching the repo's existing "thin cmd/, real code under internal/" convention.
    • Each file now reads top-to-bottom as one responsibility.
  2. Pruned dead flags.

    • Several flags had been copy-pasted from mirror and were never wired up. (removed them)
    • Help text no longer advertises options that do nothing.
  3. Documented the plugin model itself.

    • A package-level doc.go lays out what a d8 plugin actually is
    • A reader can now form a mental model of the system from one place, before opening any individual file.
  4. Promoted filesystem layout to a first-class concern.

    • A new internal/plugins/cmd/layout package became the single source of truth for paths:
      • plugins root
      • current symlink
      • contract file
      • and so on...
    • Every command routes through it instead of joining paths inline.
    • We want exactly one place to update if the on-disk shape evolves.
  5. Extracted install/run helpers.

    • The install code used to be one long function that called os.Exit inline whenever anything went wrong - which made it impossible to unit-test and awkward to extend.
    • It was split into small named helpers that return errors instead.
    • The top-level command now reads as a short sequence of calls, and the implementation stage gets a clean place to plug in "validate cluster-side requirements before launching the plugin".

Verification

  • go build ./... clean at every commit.

Follow-up

PR [2/2] (the actual contract implementation) is stacked on this branch and will be opened against it.

…to internal

This is for consistency with other commands and their layouts

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
Some of them are likely copied from mirror command, but in plugins there is no use of them.

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
…mand and plugins in general

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
…th builder functions)

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
- `runInstalledPlugin` returns errors, so the run path is testable and the command body just logs and exits on failure.
- `ensureInstallRoot` centralizes the homedir fallback used by both `NewCommand` and `NewPluginCommand`.
- `cachedDescription` wraps the contract cache lookup and returns "" when the file is missing or unreadable.
- All path joins flow through the `layout` package (`PluginsRoot`, `CurrentLinkPath`, `ContractFile`).

Signed-off-by: Roman Berezkin <roman.berezkin@flant.com>
@Glitchy-Sheep Glitchy-Sheep self-assigned this May 12, 2026
@Glitchy-Sheep Glitchy-Sheep added the enhancement New feature or request label May 12, 2026
@Glitchy-Sheep Glitchy-Sheep marked this pull request as ready for review May 12, 2026 16:17
@Glitchy-Sheep Glitchy-Sheep requested a review from ldmonster as a code owner May 12, 2026 16:17
@ldmonster ldmonster merged commit abd0f76 into main May 13, 2026
9 checks passed
@ldmonster ldmonster deleted the feat/new-plugins-contract-stage1-refactor branch May 13, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants