From 1dfae3c48c0daed3ae1b451d1c994f5c6e0f3527 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 1 Jul 2026 16:25:06 -0500 Subject: [PATCH] fix(cli): emit plugin manifest via datumctl SDK so install succeeds `datumctl plugin install milo-os/ipam` failed with: parse plugin manifest: json: cannot unmarshal string into Go struct field PluginManifest.min_api_version of type int The milo-ipam binary hand-rolled its own copy of datumctl's plugin manifest struct and declared `min_api_version` as a string holding the Kubernetes API group/version ("ipam.miloapis.com/v1alpha1"). datumctl's contract defines `min_api_version` as an integer plugin-contract version, so unmarshalling the emitted manifest failed and the plugin could not be installed. Adopt datumctl's plugin SDK instead of duplicating the contract: - Build `plugin.Manifest` and serve it via `plugin.ServeManifest` in main(), so the datumctl <-> plugin manifest contract is enforced by the compiler and cannot drift. (The SDK is already a dependency, used by transport.go and entitlement.go.) - `min_api_version` is now the integer contract version (1); the IPAM API group/version stays as a human-facing string shown by `version`. - Drop the redundant `--plugin-manifest` cobra flag; the SDK intercepts it before cobra runs. Requires a patch release for the fix to reach `datumctl plugin install`, which pulls the published binary. --- cmd/milo-ipam/main.go | 6 +++++ cmd/milo-ipam/manifest.go | 49 ++++++++++++++++-------------------- cmd/milo-ipam/output_test.go | 22 +++++++++------- cmd/milo-ipam/root.go | 14 +++-------- 4 files changed, 43 insertions(+), 48 deletions(-) diff --git a/cmd/milo-ipam/main.go b/cmd/milo-ipam/main.go index bbc98af..2c72b16 100644 --- a/cmd/milo-ipam/main.go +++ b/cmd/milo-ipam/main.go @@ -9,9 +9,15 @@ import ( "strings" "github.com/spf13/cobra" + "go.datum.net/datumctl/plugin" ) func main() { + // Serve --plugin-manifest via the datumctl SDK before cobra runs, so the + // manifest is emitted even if flag parsing would otherwise fail. ServeManifest + // prints the JSON and exits 0 when the flag is present; otherwise it returns. + plugin.ServeManifest(pluginManifest()) + io := stdStreams() root := newRootCommand(io) diff --git a/cmd/milo-ipam/manifest.go b/cmd/milo-ipam/manifest.go index f9d8b9e..b5921c3 100644 --- a/cmd/milo-ipam/manifest.go +++ b/cmd/milo-ipam/manifest.go @@ -1,14 +1,14 @@ package main import ( - "encoding/json" - "io" + "go.datum.net/datumctl/plugin" ) -// Plugin contract constants. datumctl's plugin SDK is internal to the datumctl -// repository and is not importable, so the thin manifest/env contract is -// reimplemented here. datumctl discovers a plugin by invoking it with -// --plugin-manifest and expects a single JSON document on stdout and exit 0. +// Plugin contract constants. The manifest document itself is defined by +// datumctl's plugin SDK (plugin.Manifest); this binary builds one and lets +// plugin.ServeManifest handle the --plugin-manifest protocol. Using the SDK +// type means the datumctl <-> plugin contract is enforced by the compiler +// rather than duplicated by hand. const ( pluginName = "ipam" @@ -19,8 +19,16 @@ const ( // minDatumctlVersion is the lowest datumctl that knows how to dispatch to // this plugin. minDatumctlVersion = "0.5.0" - // minAPIVersion is the IPAM apiserver API group/version this plugin targets. - minAPIVersion = "ipam.miloapis.com/v1alpha1" + // minAPIVersion is the lowest datumctl <-> plugin contract version this + // binary can run against. datumctl hard-blocks a plugin whose min_api_version + // exceeds the host's contract version. This is an integer contract version, + // NOT a Kubernetes API group/version. + minAPIVersion = pluginAPIVersion + // ipamAPIGroupVersion is the IPAM apiserver group/version this plugin talks + // to. It is human-facing (shown by `version`) and is deliberately kept out of + // the datumctl plugin manifest, whose api_version fields are integer contract + // versions. + ipamAPIGroupVersion = "ipam.miloapis.com/v1alpha1" ) // pluginVersion is the plugin's release version. It defaults to "0.0.0" to mark @@ -30,18 +38,11 @@ const ( // a var (not a const) precisely so the linker can set it. var pluginVersion = "0.0.0" -// pluginManifest is the document emitted in response to --plugin-manifest. -type pluginManifest struct { - Name string `json:"name"` - Version string `json:"version"` - Description string `json:"description"` - APIVersion int `json:"api_version"` - MinDatumctlVersion string `json:"min_datumctl_version"` - MinAPIVersion string `json:"min_api_version"` -} - -func defaultManifest() pluginManifest { - return pluginManifest{ +// pluginManifest builds the manifest datumctl reads via --plugin-manifest. The +// return type is the SDK's plugin.Manifest, so field names and types stay in +// lockstep with the host contract. +func pluginManifest() plugin.Manifest { + return plugin.Manifest{ Name: pluginName, Version: pluginVersion, Description: pluginDescription, @@ -50,11 +51,3 @@ func defaultManifest() pluginManifest { MinAPIVersion: minAPIVersion, } } - -// writeManifest renders the manifest as indented JSON. Returned separately from -// printing so it can be unit tested. -func writeManifest(w io.Writer, m pluginManifest) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - return enc.Encode(m) -} diff --git a/cmd/milo-ipam/output_test.go b/cmd/milo-ipam/output_test.go index 549ce26..fda95e2 100644 --- a/cmd/milo-ipam/output_test.go +++ b/cmd/milo-ipam/output_test.go @@ -86,16 +86,20 @@ func TestEncodeJSONIsValidAndClean(t *testing.T) { } } -func TestManifestWriter(t *testing.T) { - var buf bytes.Buffer - if err := writeManifest(&buf, defaultManifest()); err != nil { - t.Fatal(err) +func TestPluginManifest(t *testing.T) { + m := pluginManifest() + if m.Name != "ipam" || m.APIVersion != 1 || m.MinAPIVersion != 1 { + t.Fatalf("unexpected manifest: %+v", m) } - var m pluginManifest - if err := json.Unmarshal(buf.Bytes(), &m); err != nil { - t.Fatalf("manifest is not valid JSON: %v", err) + + // Guard the datumctl contract: api_version fields are integers. Emitting + // min_api_version as a JSON string is what broke `datumctl plugin install` + // (it unmarshals into an int field). Serialize and assert the numeric form. + b, err := json.Marshal(m) + if err != nil { + t.Fatal(err) } - if m.Name != "ipam" || m.APIVersion != 1 || m.MinAPIVersion == "" { - t.Fatalf("unexpected manifest: %+v", m) + if !strings.Contains(string(b), `"min_api_version":1`) { + t.Fatalf("min_api_version must serialize as an integer, got: %s", b) } } diff --git a/cmd/milo-ipam/root.go b/cmd/milo-ipam/root.go index 12d176f..ccb87b9 100644 --- a/cmd/milo-ipam/root.go +++ b/cmd/milo-ipam/root.go @@ -12,8 +12,6 @@ func newRootCommand(io IOStreams) *cobra.Command { opts := &globalOptions{output: outputTable, color: "auto"} a := newApp(io, opts) - var showManifest bool - root := &cobra.Command{ Use: "ipam", Short: "Manage IP address space (pools and prefixes) on Datum", @@ -34,17 +32,10 @@ scripts (data on stdout, diagnostics on stderr). Exit codes are documented and distinct per failure class (notably 7 = IPAM_POOL_EXHAUSTED).`, SilenceUsage: true, SilenceErrors: true, - // Intercept --plugin-manifest before any subcommand dispatch. RunE: func(cmd *cobra.Command, args []string) error { - if showManifest { - return writeManifest(io.Out, defaultManifest()) - } return cmd.Help() }, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - if showManifest { - return nil - } if !isValidOutput(opts.output) { return usageErrorf("invalid -o %q: must be one of %v", opts.output, validOutputs()) } @@ -66,8 +57,9 @@ distinct per failure class (notably 7 = IPAM_POOL_EXHAUSTED).`, }, } + // Note: --plugin-manifest is handled by plugin.ServeManifest in main() before + // cobra runs, so it is intentionally not registered as a cobra flag here. pf := root.PersistentFlags() - pf.BoolVar(&showManifest, "plugin-manifest", false, "Print the plugin manifest as JSON and exit") pf.StringVar(&opts.kubeconfig, "kubeconfig", "", "Path to a kubeconfig (forces kubeconfig transport; for dev/e2e clusters)") pf.StringVarP(&opts.namespace, "namespace", "n", "", "Namespace for namespaced resources (claims/allocations); defaults to the active context") pf.StringVarP(&opts.output, "output", "o", outputTable, "Output format: table|wide|json|yaml|name") @@ -109,7 +101,7 @@ func newVersionCommand(io IOStreams) *cobra.Command { Short: "Print the plugin version", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - _, _ = fmt.Fprintf(io.Out, "milo-ipam %s (IPAM API %s)\n", pluginVersion, minAPIVersion) + _, _ = fmt.Fprintf(io.Out, "milo-ipam %s (IPAM API %s)\n", pluginVersion, ipamAPIGroupVersion) return nil }, }