fix(cli): make the IPAM CLI plugin installable via datumctl

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)
 

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)
-}

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)
 	}
 }

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
 		},
 	}