Skip to content

Commit 83582d0

Browse files
scotwellsclaude
andcommitted
fix(plugin): catalog version pinning and command UX cleanup
Route an install argument by stripping any @Version suffix first, then deciding catalog/name vs owner/repo on the remainder, so catalog/name@ver installs the pinned version from the catalog instead of being misrouted to GitHub, and a bare name@version resolves across catalogs at that version. The version is now threaded through to the install instead of dropped. A third-party 'index add' in a non-interactive shell without --yes now fails loudly instead of silently succeeding without adding the catalog, so a CI script that forgot --yes is not misled; an explicit "no" still aborts cleanly. The trust prompt's wording matches the actual hard block. Wrap command help in the shared templates helpers, align the search and list columns, keep empty-result listings on stdout, render the collision hint as aligned columns, and drop internal artifacts (plugins.json, --plugin-manifest, api_version) from user-facing help text. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 788840d commit 83582d0

11 files changed

Lines changed: 315 additions & 135 deletions

File tree

internal/cmd/plugin/browse.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/spf13/cobra"
1010
"golang.org/x/term"
1111
componentversion "k8s.io/component-base/version"
12+
"k8s.io/kubectl/pkg/util/templates"
1213

1314
customerrors "go.datum.net/datumctl/internal/errors"
1415
"go.datum.net/datumctl/internal/pluginstore"
@@ -26,17 +27,19 @@ func browseCmd() *cobra.Command {
2627
cmd := &cobra.Command{
2728
Use: "browse",
2829
Short: "Browse and install plugins interactively",
29-
Long: `Open an interactive browser over every registered plugin catalog.
30-
31-
Type to filter, inspect a plugin's description, version, source catalog, and
32-
trust badge ("official" for Datum's curated datum catalog, "third-party" for
33-
catalogs you added), and install the selected plugin in place. Use --index to
34-
scope the browser to a single catalog.`,
35-
Example: ` # Browse all catalogs
36-
datumctl plugin browse
37-
38-
# Browse a single catalog
39-
datumctl plugin browse --index acme`,
30+
Long: templates.LongDesc(`
31+
Open an interactive browser over every registered plugin catalog.
32+
33+
Type to filter, inspect a plugin's description, version, source catalog, and
34+
trust badge ("official" for Datum's curated datum catalog, "third-party" for
35+
catalogs you added), and install the selected plugin in place. Use --index to
36+
scope the browser to a single catalog.`),
37+
Example: templates.Examples(`
38+
# Browse all catalogs
39+
datumctl plugin browse
40+
41+
# Browse a single catalog
42+
datumctl plugin browse --index acme`),
4043
Args: cobra.NoArgs,
4144
RunE: func(cmd *cobra.Command, args []string) error {
4245
if !browseIsTerminal() {
@@ -92,7 +95,8 @@ scope the browser to a single catalog.`,
9295
}
9396

9497
currentVersion := componentversion.Get().GitVersion
95-
return installFromCatalog(cmd, pluginsDir, reg, selected.catalog.Name, selected.plugin.Name, currentVersion)
98+
// Empty version installs the catalog's recommended release.
99+
return installFromCatalog(cmd, pluginsDir, reg, selected.catalog.Name, selected.plugin.Name, "", currentVersion)
96100
},
97101
}
98102
cmd.Flags().StringVar(&indexName, "index", "", "Scope the browser to a single catalog")

internal/cmd/plugin/catalog_resolve.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,28 @@ func resolveBareName(cmd *cobra.Command, pluginsDir string, reg *pluginstore.Reg
7979

8080
// collisionError renders the "available from multiple catalogs" message that
8181
// asks the user to qualify a bare name. Matches are listed default-first.
82+
// Columns are padded to a common width so they align when the message is
83+
// rendered as a plain string (UserError does not run it through a tabwriter).
8284
func collisionError(name string, matches []catalogMatch) error {
85+
cmds := make([]string, len(matches))
86+
descs := make([]string, len(matches))
87+
cmdWidth, descWidth := 0, 0
88+
for i, m := range matches {
89+
cmds[i] = fmt.Sprintf("datumctl plugin install %s/%s", m.catalog.Name, name)
90+
descs[i] = m.plugin.Spec.ShortDescription
91+
if len(cmds[i]) > cmdWidth {
92+
cmdWidth = len(cmds[i])
93+
}
94+
if len(descs[i]) > descWidth {
95+
descWidth = len(descs[i])
96+
}
97+
}
98+
8399
var b strings.Builder
84100
fmt.Fprintf(&b, "%q is available from multiple catalogs. Choose one:", name)
85-
for _, m := range matches {
86-
desc := m.plugin.Spec.ShortDescription
87-
fmt.Fprintf(&b, "\n datumctl plugin install %s/%s\t%s\t(%s)",
88-
m.catalog.Name, name, desc, m.catalog.Trust())
101+
for i, m := range matches {
102+
fmt.Fprintf(&b, "\n %-*s %-*s (%s)",
103+
cmdWidth, cmds[i], descWidth, descs[i], m.catalog.Trust())
89104
}
90105
return fmt.Errorf("%s", b.String())
91106
}

internal/cmd/plugin/index.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,19 @@ func indexAddCmd() *cobra.Command {
130130

131131
// One-time trust decision for third-party catalogs.
132132
if !assumeYes {
133-
ok, promptErr := confirmCatalogTrust(cmd, name)
133+
answered, ok, promptErr := confirmCatalogTrust(cmd, name)
134134
if promptErr != nil {
135135
return promptErr
136136
}
137+
if !answered {
138+
// No one answered the prompt (e.g. non-interactive shell, EOF).
139+
// Fail loudly (non-zero) rather than silently succeeding without
140+
// adding the catalog, so a CI script that forgot --yes is not misled.
141+
return customerrors.NewUserErrorWithHint(
142+
fmt.Sprintf("adding third-party catalog %q requires confirmation", name),
143+
"Re-run with --yes to confirm without a prompt (for scripts/CI).",
144+
)
145+
}
137146
if !ok {
138147
fmt.Fprintln(cmd.OutOrStdout(), "Aborted. No catalog was added.")
139148
return nil
@@ -389,23 +398,27 @@ func indexValidateCmd() *cobra.Command {
389398
}
390399

391400
// confirmCatalogTrust shows the one-time third-party trust explanation and reads
392-
// a y/N answer from stdin.
393-
func confirmCatalogTrust(cmd *cobra.Command, name string) (bool, error) {
394-
out := cmd.OutOrStdout()
401+
// a y/N answer from stdin. answered reports whether the user actually responded;
402+
// it is false when the prompt receives no input (EOF / non-interactive), letting
403+
// the caller distinguish an unanswered prompt from an explicit "no".
404+
func confirmCatalogTrust(cmd *cobra.Command, name string) (answered, confirmed bool, err error) {
405+
// Interactive prompts go to stderr so stdout stays clean for redirection.
406+
out := cmd.ErrOrStderr()
395407
fmt.Fprintf(out, "\n You're adding a third-party plugin catalog: %q\n\n", name)
396408
fmt.Fprintln(out, " Datum does not review, verify, or endorse plugins from this catalog.")
397409
fmt.Fprintln(out, " Plugins are programs that run on your machine with your Datum credentials.")
398410
fmt.Fprintln(out, " Only add catalogs you trust.")
399411
fmt.Fprint(out, "\n Add this catalog? [y/N] ")
400412

401413
reader := bufio.NewReader(cmd.InOrStdin())
402-
line, err := reader.ReadString('\n')
403-
if err != nil && line == "" {
404-
// EOF with no input (e.g. non-interactive) is treated as "no".
405-
return false, nil
414+
line, readErr := reader.ReadString('\n')
415+
if readErr != nil && line == "" {
416+
// EOF with no input (e.g. non-interactive shell): the prompt went
417+
// unanswered. Leave the decision to the caller.
418+
return false, false, nil
406419
}
407420
answer := strings.ToLower(strings.TrimSpace(line))
408-
return answer == "y" || answer == "yes", nil
421+
return true, answer == "y" || answer == "yes", nil
409422
}
410423

411424
// updateCatalogHeader copies the freshly fetched catalog header into the

internal/cmd/plugin/index_test.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,19 @@ func TestIndexAdd_trustPromptDecline(t *testing.T) {
110110
}
111111
}
112112

113-
func TestIndexAdd_emptyStdinDeclines(t *testing.T) {
113+
func TestIndexAdd_emptyStdinErrors(t *testing.T) {
114114
pluginsDir := t.TempDir()
115115
src := writeLocalCatalog(t, testCatalogManifest)
116116

117-
// No stdin -> EOF -> treated as "no".
117+
// No stdin -> the prompt goes unanswered. Rather than silently succeeding
118+
// (or silently aborting) we fail loudly so a CI script that forgot --yes is
119+
// not misled into thinking the catalog was added.
118120
_, err := runIndex(t, pluginsDir, "", "add", "acme", src)
119-
if err != nil {
120-
t.Fatalf("unexpected error: %v", err)
121+
if err == nil {
122+
t.Fatal("expected an error when the trust prompt gets no confirmation")
123+
}
124+
if !strings.Contains(err.Error(), "requires confirmation") {
125+
t.Fatalf("expected a 'requires confirmation' error, got: %v", err)
121126
}
122127
if reg, _ := pluginstore.LoadRegistry(pluginsDir); reg.Find("acme") != nil {
123128
t.Fatal("acme must NOT be registered when prompt gets no confirmation")

internal/cmd/plugin/install.go

Lines changed: 88 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,16 @@ func installCmd(_ *client.DatumCloudFactory) *cobra.Command {
2323
Short: "Install a datumctl plugin",
2424
Long: `Install a datumctl plugin from a plugin catalog or a GitHub Release.
2525
26-
With no arguments, restores all plugins recorded in plugins.json to their
27-
recorded versions. Use this to reproduce a plugin set on a new machine.
26+
With no arguments, restores all previously installed plugins to their recorded
27+
versions. Use this to reproduce a plugin set on a new machine.
2828
2929
With a plugin name argument, installs from a registered catalog:
30-
- name resolves against the official datum catalog first, then any
31-
other registered catalog; a name found in more than one
32-
catalog prints the options instead of guessing
33-
- catalog/name installs from a specific registered catalog (e.g. datum/dns)
30+
- name resolves against the official datum catalog first, then
31+
any other registered catalog; a name found in more than
32+
one catalog prints the options instead of guessing
33+
- name@version installs a specific version of a catalog plugin
34+
- catalog/name installs from a specific registered catalog (e.g. datum/dns)
35+
- catalog/name@version installs a specific version from a specific catalog
3436
3537
With an owner/repo argument, installs directly from a GitHub Release:
3638
- owner/repo installs the latest release
@@ -50,7 +52,11 @@ The plugin binary is written to the managed plugins directory
5052
# Install a specific version from GitHub
5153
datumctl plugin install datum-cloud/datumctl-dns@v1.2.0
5254
53-
# Restore all plugins from plugins.json
55+
# Install a specific version of a catalog plugin
56+
datumctl plugin install dns@v1.2.0
57+
datumctl plugin install acme/deploy@v2.0.0
58+
59+
# Restore all previously installed plugins
5460
datumctl plugin install`,
5561
RunE: func(cmd *cobra.Command, args []string) error {
5662
pluginsDir, err := resolvePluginsDir(cmd)
@@ -87,31 +93,77 @@ The plugin binary is written to the managed plugins directory
8793
return cmd
8894
}
8995

90-
// installArg dispatches a single install argument to the right path based on
91-
// catalog-aware addressing:
92-
// - "owner/repo@version" -> direct GitHub release (the '@' is unambiguous)
93-
// - "catalog/name" -> catalog-qualified, when the prefix is a
94-
// registered catalog name
95-
// - "owner/repo" -> direct GitHub release otherwise
96-
// - "name" -> bare name, resolved across catalogs (default
97-
// first); a collision prints the qualified options instead of guessing
98-
func installArg(cmd *cobra.Command, pluginsDir string, reg *pluginstore.Registry, arg, currentVersion string) error {
99-
if strings.Contains(arg, "@") {
100-
return installGitHubArg(cmd, pluginsDir, arg, currentVersion)
101-
}
102-
if strings.Contains(arg, "/") {
103-
prefix, rest, _ := strings.Cut(arg, "/")
96+
// installRouteKind identifies which install path an argument resolves to.
97+
type installRouteKind int
98+
99+
const (
100+
// routeGitHub installs directly from a GitHub release (owner/repo).
101+
routeGitHub installRouteKind = iota
102+
// routeCatalog installs a named plugin from a specific registered catalog.
103+
routeCatalog
104+
// routeBare resolves a bare plugin name across all registered catalogs.
105+
routeBare
106+
)
107+
108+
// installRoute is the parsed routing decision for a single install argument.
109+
// It is produced by routeInstallArg and consumed by installArg; keeping the
110+
// decision in a pure value makes the routing logic unit-testable without
111+
// performing real network installs.
112+
type installRoute struct {
113+
kind installRouteKind
114+
// ghSource is the "owner/repo" (or "github.com/owner/repo") string to hand
115+
// to the GitHub install path; only set for routeGitHub.
116+
ghSource string
117+
// catalog is the registered catalog name; only set for routeCatalog.
118+
catalog string
119+
// name is the plugin name (catalog/bare paths); unset for routeGitHub.
120+
name string
121+
// version is the trailing "@version" requested, or "" for latest. It is
122+
// threaded through every route.
123+
version string
124+
}
125+
126+
// routeInstallArg parses a single install argument into a routing decision.
127+
// Any trailing "@version" is split off FIRST, then routing is decided on the
128+
// remaining string so that version-pinning works uniformly across catalogs and
129+
// GitHub:
130+
// - "catalog/name" -> routeCatalog, when the prefix is a registered catalog
131+
// - "owner/repo" -> routeGitHub otherwise (the slash is owner/repo)
132+
// - "name" -> routeBare, resolved across catalogs (default first)
133+
//
134+
// The version is carried on the route so "catalog/name@v2", "name@v2", and
135+
// "owner/repo@v2" all pin the requested version through their respective paths.
136+
func routeInstallArg(reg *pluginstore.Registry, arg string) installRoute {
137+
rest, version, _ := strings.Cut(arg, "@")
138+
if strings.Contains(rest, "/") {
139+
prefix, name, _ := strings.Cut(rest, "/")
104140
if reg.Find(prefix) != nil {
105-
return installFromCatalog(cmd, pluginsDir, reg, prefix, rest, currentVersion)
141+
return installRoute{kind: routeCatalog, catalog: prefix, name: name, version: version}
106142
}
107-
return installGitHubArg(cmd, pluginsDir, arg, currentVersion)
143+
return installRoute{kind: routeGitHub, ghSource: rest, version: version}
144+
}
145+
return installRoute{kind: routeBare, name: rest, version: version}
146+
}
147+
148+
// installArg dispatches a single install argument to the right path based on
149+
// catalog-aware addressing. See routeInstallArg for the routing rules.
150+
func installArg(cmd *cobra.Command, pluginsDir string, reg *pluginstore.Registry, arg, currentVersion string) error {
151+
route := routeInstallArg(reg, arg)
152+
switch route.kind {
153+
case routeCatalog:
154+
return installFromCatalog(cmd, pluginsDir, reg, route.catalog, route.name, route.version, currentVersion)
155+
case routeBare:
156+
return installBareName(cmd, pluginsDir, reg, route.name, route.version, currentVersion)
157+
default:
158+
return installGitHubArg(cmd, pluginsDir, route.ghSource, route.version, currentVersion)
108159
}
109-
return installBareName(cmd, pluginsDir, reg, arg, currentVersion)
110160
}
111161

112-
// installGitHubArg installs directly from a GitHub release (owner/repo[@version]).
113-
func installGitHubArg(cmd *cobra.Command, pluginsDir, arg, currentVersion string) error {
114-
owner, repo, version, parseErr := parseSource(arg)
162+
// installGitHubArg installs directly from a GitHub release. ghSource is the
163+
// "owner/repo" (or "github.com/owner/repo") string with any "@version" already
164+
// split off; the requested version is passed separately.
165+
func installGitHubArg(cmd *cobra.Command, pluginsDir, ghSource, version, currentVersion string) error {
166+
owner, repo, _, parseErr := parseSource(ghSource)
115167
if parseErr != nil {
116168
return customerrors.NewUserError(parseErr.Error())
117169
}
@@ -122,8 +174,10 @@ func installGitHubArg(cmd *cobra.Command, pluginsDir, arg, currentVersion string
122174
return saveAndReport(cmd, pluginsDir, pluginName, entry, binaryPath)
123175
}
124176

125-
// installFromCatalog installs a named plugin from a specific registered catalog.
126-
func installFromCatalog(cmd *cobra.Command, pluginsDir string, reg *pluginstore.Registry, catalogName, pluginName, currentVersion string) error {
177+
// installFromCatalog installs a named plugin from a specific registered
178+
// catalog. version pins a specific release, or "" installs the catalog's
179+
// recommended version.
180+
func installFromCatalog(cmd *cobra.Command, pluginsDir string, reg *pluginstore.Registry, catalogName, pluginName, version, currentVersion string) error {
127181
cat := reg.Find(catalogName)
128182
if cat == nil {
129183
return customerrors.NewUserError(fmt.Sprintf("catalog %q is not registered; run 'datumctl plugin index add %s <source>' first", catalogName, catalogName))
@@ -132,7 +186,7 @@ func installFromCatalog(cmd *cobra.Command, pluginsDir string, reg *pluginstore.
132186
if err != nil {
133187
return indexFetchUserError(err)
134188
}
135-
idxEntry, name, binaryPath, installErr := installPlugin(cmd.Context(), pluginsDir, pluginName, "", currentVersion, idx)
189+
idxEntry, name, binaryPath, installErr := installPlugin(cmd.Context(), pluginsDir, pluginName, version, currentVersion, idx)
136190
if installErr != nil {
137191
return customerrors.NewUserError(fmt.Sprintf("install plugin %s/%s: %v", catalogName, pluginName, installErr))
138192
}
@@ -144,13 +198,14 @@ func installFromCatalog(cmd *cobra.Command, pluginsDir string, reg *pluginstore.
144198

145199
// installBareName resolves a bare plugin name across all catalogs (default
146200
// first) and installs the unique match, or surfaces the options on a collision.
147-
func installBareName(cmd *cobra.Command, pluginsDir string, reg *pluginstore.Registry, name, currentVersion string) error {
201+
// version pins a specific release, or "" installs the recommended version.
202+
func installBareName(cmd *cobra.Command, pluginsDir string, reg *pluginstore.Registry, name, version, currentVersion string) error {
148203
matches := resolveBareName(cmd, pluginsDir, reg, name)
149204
switch len(matches) {
150205
case 0:
151206
return customerrors.NewUserError(fmt.Sprintf("plugin %q not found in any registered catalog; run 'datumctl plugin search %s' to look for it", name, name))
152207
case 1:
153-
return installFromCatalog(cmd, pluginsDir, reg, matches[0].catalog.Name, name, currentVersion)
208+
return installFromCatalog(cmd, pluginsDir, reg, matches[0].catalog.Name, name, version, currentVersion)
154209
default:
155210
return customerrors.NewUserError(collisionError(name, matches).Error())
156211
}
@@ -199,7 +254,7 @@ func installAllFromManifest(cmd *cobra.Command, pluginsDir, currentVersion strin
199254
}
200255

201256
if len(manifest.Plugins) == 0 {
202-
fmt.Fprintln(cmd.OutOrStdout(), "No plugins recorded in plugins.json.")
257+
fmt.Fprintln(cmd.OutOrStdout(), "No previously installed plugins to restore.")
203258
return nil
204259
}
205260

0 commit comments

Comments
 (0)