From 05f390e618e609dc948845fb4950534b60738f33 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Wed, 7 Oct 2026 13:55:59 +1000 Subject: [PATCH] refactor: move spx command tree to operator/cli Moving the command tree out of cmd/ brings it under errcheck and gosec. Flag and config wiring now fails loudly instead of silently dropping a binding, listing commands fail when they cannot write their table, and the formation server's unauthenticated TLS is annotated as a known gap. --- Makefile | 2 +- build/Dockerfile.distro | 4 +- cmd/spinifex/main.go | 4 +- scripts/reset-dev-env.sh | 2 +- .../network/invariants/listener_bind_test.go | 5 +- spinifex/gateway/bedrockagent_test.go | 2 +- spinifex/migrate/VERSIONS.md | 10 +- .../cmd => spinifex/operator/cli}/admin.go | 47 ++++--- .../operator/cli}/admin_account_delete.go | 2 +- .../cli}/admin_account_delete_output_test.go | 14 +- .../cli}/admin_account_delete_remote.go | 2 +- .../cli}/admin_account_delete_remote_test.go | 46 +++---- .../cli}/admin_account_list_remote.go | 2 +- .../operator/cli}/admin_account_quota.go | 2 +- .../operator/cli}/admin_account_quota_test.go | 2 +- .../operator/cli}/admin_acm.go | 6 +- .../operator/cli}/admin_eks.go | 4 +- .../operator/cli}/admin_firewall.go | 2 +- .../operator/cli}/admin_firewall_test.go | 14 +- .../operator/cli}/admin_gpu.go | 14 +- .../operator/cli}/admin_jsprobe.go | 2 +- .../operator/cli}/admin_jsprobe_test.go | 2 +- .../cmd => spinifex/operator/cli}/admin_kv.go | 2 +- .../operator/cli}/admin_kv_compare.go | 2 +- .../operator/cli}/admin_kv_compare_test.go | 48 +++---- .../operator/cli}/admin_kv_replicas.go | 2 +- .../operator/cli}/admin_kv_replicas_test.go | 2 +- .../operator/cli}/admin_kv_test.go | 2 +- .../operator/cli}/admin_nat_test.go | 2 +- .../operator/cli}/admin_node_count_test.go | 2 +- .../operator/cli}/admin_ochre.go | 18 +-- .../operator/cli}/admin_ochre_appliance.go | 2 +- .../cli}/admin_ochre_appliance_test.go | 2 +- .../operator/cli}/admin_ochre_endpoint.go | 8 +- .../cli}/admin_ochre_endpoint_test.go | 2 +- .../operator/cli}/admin_ochre_pull_test.go | 2 +- .../operator/cli}/admin_ochre_test.go | 2 +- .../operator/cli}/admin_ochre_vector.go | 20 +-- .../operator/cli}/admin_ochre_vector_test.go | 2 +- .../operator/cli}/admin_preflight.go | 2 +- .../operator/cli}/admin_principal.go | 2 +- .../operator/cli}/admin_principal_iam_test.go | 18 +-- .../operator/cli}/admin_principal_test.go | 46 +++---- .../operator/cli}/admin_remote.go | 8 +- .../operator/cli}/admin_remote_test.go | 38 ++--- .../operator/cli}/admin_test.go | 2 +- .../operator/cli}/admin_upgrade.go | 2 +- .../operator/cli}/admin_upgrade_test.go | 2 +- .../cmd => spinifex/operator/cli}/cluster.go | 2 +- .../operator/cli}/cluster_test.go | 2 +- .../operator/cli}/export_test.go | 2 +- .../cmd => spinifex/operator/cli}/get.go | 22 ++- .../operator/cli}/get_dhcp_leases.go | 2 +- .../operator/cli}/get_dhcp_leases_test.go | 2 +- .../cmd => spinifex/operator/cli}/get_test.go | 2 +- .../operator/cli}/jetstream_store.go | 2 +- .../operator/cli}/jetstream_store_test.go | 2 +- .../operator/cli}/nats_server_name_test.go | 2 +- .../operator/cli}/northstar_template_test.go | 2 +- .../cmd => spinifex/operator/cli}/paths.go | 2 +- .../operator/cli}/paths_test.go | 2 +- .../operator/cli}/predastore_bind_test.go | 2 +- .../operator/cli}/predastore_planes_test.go | 2 +- .../register_imported_ami_snapshot_test.go | 2 +- .../cmd => spinifex/operator/cli}/root.go | 35 +++-- .../operator/cli}/root_logger_test.go | 12 +- .../cmd => spinifex/operator/cli}/service.go | 130 +++++++++--------- .../cli}/service_bind_collision_test.go | 2 +- .../cli}/service_console_region_test.go | 10 +- .../operator/cli}/service_exit_test.go | 2 +- .../operator/cli}/service_test.go | 2 +- .../operator/cli}/telemetry_test.go | 2 +- .../operator/cli}/templates/awsgw.toml | 0 .../operator/cli}/templates/nats.conf | 0 .../operator/cli}/templates/northstar.toml | 0 .../cli}/templates/predastore-multinode.toml | 0 .../operator/cli}/templates/predastore.toml | 0 .../operator/cli}/templates/spinifex.toml | 0 .../cmd => spinifex/operator/cli}/top.go | 22 ++- .../cmd => spinifex/operator/cli}/version.go | 4 +- .../operator/cli}/viperblock_region_test.go | 2 +- spinifex/operator/cli/wiring.go | 33 +++++ spinifex/operator/cli/wiring_test.go | 24 ++++ spinifex/runtime/roles/nats/nats_test.go | 2 +- 84 files changed, 432 insertions(+), 332 deletions(-) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin.go (98%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_account_delete.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_account_delete_output_test.go (92%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_account_delete_remote.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_account_delete_remote_test.go (82%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_account_list_remote.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_account_quota.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_account_quota_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_acm.go (98%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_eks.go (97%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_firewall.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_firewall_test.go (86%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_gpu.go (96%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_jsprobe.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_jsprobe_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_kv.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_kv_compare.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_kv_compare_test.go (83%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_kv_replicas.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_kv_replicas_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_kv_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_nat_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_node_count_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre_appliance.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre_appliance_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre_endpoint.go (98%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre_endpoint_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre_pull_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre_vector.go (97%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_ochre_vector_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_preflight.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_principal.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_principal_iam_test.go (90%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_principal_test.go (82%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_remote.go (98%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_remote_test.go (90%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_upgrade.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/admin_upgrade_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/cluster.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/cluster_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/export_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/get.go (91%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/get_dhcp_leases.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/get_dhcp_leases_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/get_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/jetstream_store.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/jetstream_store_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/nats_server_name_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/northstar_template_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/paths.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/paths_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/predastore_bind_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/predastore_planes_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/register_imported_ami_snapshot_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/root.go (81%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/root_logger_test.go (88%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/service.go (88%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/service_bind_collision_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/service_console_region_test.go (80%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/service_exit_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/service_test.go (99%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/telemetry_test.go (97%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/templates/awsgw.toml (100%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/templates/nats.conf (100%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/templates/northstar.toml (100%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/templates/predastore-multinode.toml (100%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/templates/predastore.toml (100%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/templates/spinifex.toml (100%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/top.go (83%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/version.go (79%) rename {cmd/spinifex/cmd => spinifex/operator/cli}/viperblock_region_test.go (99%) create mode 100644 spinifex/operator/cli/wiring.go create mode 100644 spinifex/operator/cli/wiring_test.go diff --git a/Makefile b/Makefile index fc948874d..f313994cb 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,7 @@ build-ui: # GO commands VERSION ?= $(shell git describe --tags --always --dirty) COMMIT ?= $(shell git rev-parse --short HEAD) -LDFLAGS := -s -w -X github.com/mulgadc/spinifex/cmd/spinifex/cmd.Version=$(VERSION) -X github.com/mulgadc/spinifex/cmd/spinifex/cmd.Commit=$(COMMIT) +LDFLAGS := -s -w -X github.com/mulgadc/spinifex/spinifex/operator/cli.Version=$(VERSION) -X github.com/mulgadc/spinifex/spinifex/operator/cli.Commit=$(COMMIT) go_build: @echo -e "\n....Building $(GO_PROJECT_NAME)" diff --git a/build/Dockerfile.distro b/build/Dockerfile.distro index 3b21be62e..54564b8a0 100644 --- a/build/Dockerfile.distro +++ b/build/Dockerfile.distro @@ -35,8 +35,8 @@ COPY bluebottle/ ./bluebottle/ # Build spinifex binary (spx) RUN COMMIT=$(cd spinifex && git rev-parse --short HEAD 2>/dev/null || echo "unknown") && \ cd spinifex && GOFIPS140=v1.0.0 go build -ldflags "-s -w \ - -X github.com/mulgadc/spinifex/cmd/spinifex/cmd.Version=${VERSION} \ - -X github.com/mulgadc/spinifex/cmd/spinifex/cmd.Commit=${COMMIT}" \ + -X github.com/mulgadc/spinifex/spinifex/operator/cli.Version=${VERSION} \ + -X github.com/mulgadc/spinifex/spinifex/operator/cli.Commit=${COMMIT}" \ -o /out/spx cmd/spinifex/main.go # Build nbdkit viperblock plugin (.so) diff --git a/cmd/spinifex/main.go b/cmd/spinifex/main.go index 9aa442adf..574ece74f 100644 --- a/cmd/spinifex/main.go +++ b/cmd/spinifex/main.go @@ -2,9 +2,9 @@ package main import ( _ "github.com/mulgadc/bluebottle/pkg/fipsboot" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" + "github.com/mulgadc/spinifex/spinifex/operator/cli" ) func main() { - cmd.Execute() + cli.Execute() } diff --git a/scripts/reset-dev-env.sh b/scripts/reset-dev-env.sh index ad17120f8..aede4f921 100755 --- a/scripts/reset-dev-env.sh +++ b/scripts/reset-dev-env.sh @@ -273,7 +273,7 @@ if [ "$DEV_BIND_PLANES" = "1" ]; then # Pinning --bind moves every service rendered as {{.BindIP}} onto the lan # plane. awsgw and predastore must be on the wildcard first, or the public # AWS API and S3 endpoint leave the wan plane with them. - TEMPLATE="$PROJECT_ROOT/cmd/spinifex/cmd/templates/spinifex.toml" + TEMPLATE="$PROJECT_ROOT/spinifex/operator/cli/templates/spinifex.toml" if [ -f "$TEMPLATE" ] && ! grep -q '^host = "0.0.0.0:9999"' "$TEMPLATE"; then fail "DEV_BIND_PLANES=1 but $TEMPLATE still renders awsgw on {{.BindIP}}. This branch predates the wildcard public listeners — rebase, or set DEV_BIND_PLANES=0." fi diff --git a/spinifex/domains/network/invariants/listener_bind_test.go b/spinifex/domains/network/invariants/listener_bind_test.go index 6ed303ea6..cb1749684 100644 --- a/spinifex/domains/network/invariants/listener_bind_test.go +++ b/spinifex/domains/network/invariants/listener_bind_test.go @@ -93,10 +93,13 @@ func listenerBindScanFiles(t *testing.T, root string) []string { filepath.Join(root, "scripts", "install-node.sh"), filepath.Join(root, "scripts", "setup.sh"), } - tomls, err := filepath.Glob(filepath.Join(root, "cmd", "spinifex", "cmd", "templates", "*.toml")) + tomls, err := filepath.Glob(filepath.Join(root, "spinifex", "operator", "cli", "templates", "*.toml")) if err != nil { t.Fatalf("glob templates: %v", err) } + if len(tomls) == 0 { + t.Fatal("listener bind scan found no config templates") + } sort.Strings(tomls) files = append(files, tomls...) for _, f := range files { diff --git a/spinifex/gateway/bedrockagent_test.go b/spinifex/gateway/bedrockagent_test.go index ec7ba2876..c16db6ae0 100644 --- a/spinifex/gateway/bedrockagent_test.go +++ b/spinifex/gateway/bedrockagent_test.go @@ -29,7 +29,7 @@ const bedrockAgentTestAccount = "111111111111" // fakeBedrockAgentVectorService serves scripted responses so bedrockagent.go's // mapping logic can be exercised without a daemon or a live NATS connection, -// mirroring cmd/spinifex/cmd's fakeVectorService for this package. +// mirroring operator/cli's fakeVectorService for this package. type fakeBedrockAgentVectorService struct { createIndexReq *ochrevector.CreateIndexRequest createIndexErr error diff --git a/spinifex/migrate/VERSIONS.md b/spinifex/migrate/VERSIONS.md index 58abaed28..0146ae3ba 100644 --- a/spinifex/migrate/VERSIONS.md +++ b/spinifex/migrate/VERSIONS.md @@ -4,11 +4,11 @@ Single source of truth for the current schema version of every config file Spini | Target | Current version | Canonical template | |---|---|---| -| `nats.conf` | `3` | `cmd/spinifex/cmd/templates/nats.conf` | -| `awsgw.toml` | `3` | `cmd/spinifex/cmd/templates/awsgw.toml` | -| `spinifex.toml` | `4` | `cmd/spinifex/cmd/templates/spinifex.toml` | -| `predastore.toml` | `1` | `cmd/spinifex/cmd/templates/predastore.toml` | -| `predastore-multinode.toml` | `1` | `cmd/spinifex/cmd/templates/predastore-multinode.toml` | +| `nats.conf` | `3` | `spinifex/operator/cli/templates/nats.conf` | +| `awsgw.toml` | `3` | `spinifex/operator/cli/templates/awsgw.toml` | +| `spinifex.toml` | `4` | `spinifex/operator/cli/templates/spinifex.toml` | +| `predastore.toml` | `1` | `spinifex/operator/cli/templates/predastore.toml` | +| `predastore-multinode.toml` | `1` | `spinifex/operator/cli/templates/predastore-multinode.toml` | ## How versions are stamped diff --git a/cmd/spinifex/cmd/admin.go b/spinifex/operator/cli/admin.go similarity index 98% rename from cmd/spinifex/cmd/admin.go rename to spinifex/operator/cli/admin.go index 2de4d4297..758c0c0fc 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/spinifex/operator/cli/admin.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bufio" @@ -365,7 +365,7 @@ func init() { certRenewCmd.Flags().StringSlice("extra-ip", nil, "Additional IP addresses to include in SANs") certRenewCmd.Flags().StringSlice("extra-dns", nil, "Additional DNS names to include in SANs") accountCreateCmd.Flags().String("name", "", "Account name (required)") - accountCreateCmd.MarkFlagRequired("name") + mustMarkFlagRequired(accountCreateCmd, "name") rootCmd.PersistentFlags().String("config-dir", DefaultConfigDir(), "Configuration directory") rootCmd.PersistentFlags().String("spinifex-dir", DefaultDataDir(), "Spinifex base directory") @@ -424,9 +424,9 @@ func init() { adminJoinCmd.Flags().String("email", "", "Operator email address (used for update and security notifications)") adminJoinCmd.Flags().Bool("skip-host-dns", false, "Do not point this node's host resolver at its local northstar (LB/EKS names then won't resolve from the node)") adminJoinCmd.Flags().Int("predastore-compaction-interval", 0, "Predastore compactor interval in seconds (0 = unset, uses built-in default). Test clusters set a short interval.") - adminJoinCmd.MarkFlagRequired("node") - adminJoinCmd.MarkFlagRequired("host") - adminJoinCmd.MarkFlagRequired("token") + mustMarkFlagRequired(adminJoinCmd, "node") + mustMarkFlagRequired(adminJoinCmd, "host") + mustMarkFlagRequired(adminJoinCmd, "token") imagesImportCmd.Flags().String("tmp-dir", os.TempDir(), "Temporary directory for image import processing") @@ -446,14 +446,14 @@ func init() { imagesRemoveCmd.Flags().String("image-id", "", "AMI ID to remove (required)") imagesRemoveCmd.Flags().Bool("force", false, "Bypass dependency, ownership and config-corrupt checks (salvage mode)") imagesRemoveCmd.Flags().Bool("yes", false, "Skip interactive confirmation prompt") - _ = imagesRemoveCmd.MarkFlagRequired("image-id") + mustMarkFlagRequired(imagesRemoveCmd, "image-id") imagesPromoteCmd.Flags().String("image-id", "", "AMI ID to promote to system image (required)") imagesPromoteCmd.Flags().Bool("yes", false, "Skip interactive confirmation prompt") - _ = imagesPromoteCmd.MarkFlagRequired("image-id") + mustMarkFlagRequired(imagesPromoteCmd, "image-id") imagesDescribeCmd.Flags().String("image-id", "", "AMI ID to describe (required)") - _ = imagesDescribeCmd.MarkFlagRequired("image-id") + mustMarkFlagRequired(imagesDescribeCmd, "image-id") } const bytesPerGiB = 1024 * 1024 * 1024 @@ -718,7 +718,7 @@ func runimagesImportCmd(cmd *cobra.Command, args []string) { // 0770, matching the root:spinifex convention setup.sh gives the shared data // dirs. At 0700 the first sudo import leaves a directory no group member can // descend into, so every later import without sudo fails on mkdir. - if err := os.MkdirAll(imagePath, 0770); err != nil { + if err := os.MkdirAll(imagePath, 0770); err != nil { //nolint:gosec // G301: shared image directory is group-writable (0770) so later imports by other spinifex group members can create entries; it is not world-accessible. fmt.Fprintf(os.Stderr, "Error creating image directory %s: %v\n", imagePath, err) os.Exit(1) } @@ -1206,10 +1206,15 @@ func runimagesListCmd(cmd *cobra.Command, args []string) { tableData = append(tableData, []string{img.Name, img.Distro, img.Version, img.Arch, img.BootMode}) } - // Create a table with the defined data. - // The table has a header and the text in the cells is right-aligned. - // The Render() method is used to print the table to the console. - pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(tableData).Render() + // Render discards its write error, so write the table here to fail on it. + rendered, err := pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(tableData).Srender() + if err == nil { + _, err = fmt.Fprintln(os.Stdout, rendered) + } + if err != nil { + fmt.Fprintf(os.Stderr, "Error: render table: %v\n", err) + os.Exit(1) + } pterm.Println("To install a selected image as an AMI use:") @@ -2062,7 +2067,7 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, // Wait for all nodes to register if err := fs.WaitForCompletion(formationTimeout); err != nil { fmt.Fprintf(os.Stderr, "❌ %v\n", err) - fs.Shutdown(context.Background()) + _ = fs.Shutdown(context.Background()) closeFormationPort() os.Remove(tokenPath) os.Exit(1) @@ -2193,7 +2198,7 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, time.Sleep(15 * time.Second) // Shutdown formation server - fs.Shutdown(context.Background()) + _ = fs.Shutdown(context.Background()) closeFormationPort() if err := os.Remove(tokenPath); err != nil && !errors.Is(err, os.ErrNotExist) { slog.Warn("Failed to remove join token file", "path", tokenPath, "error", err) @@ -2394,7 +2399,7 @@ func runAdminJoin(cmd *cobra.Command, args []string) { client := &http.Client{ Timeout: 30 * time.Second, Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // formation server uses ephemeral self-signed cert + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // G402: KNOWN SECURITY GAP, not a false positive. The formation trust bootstrap does not authenticate the server, so the join token and returned cluster secrets are exposed to interception. Remove once formation TLS is authenticated by a pinned key/certificate or an out-of-band CA. }, } @@ -3657,12 +3662,12 @@ func installCACertificate(caPemPath string) { return } - if err := os.MkdirAll(filepath.Dir(systemCertPath), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(systemCertPath), 0755); err != nil { //nolint:gosec // G301: system CA trust-store directory must be 0755 for every TLS client to read it. Contains no secret material. fmt.Fprintf(os.Stderr, "Warning: could not create certificate directory: %v\n", err) return } - if err := os.WriteFile(systemCertPath, data, 0644); err != nil { + if err := os.WriteFile(systemCertPath, data, 0644); err != nil { //nolint:gosec // G306: public CA certificate in the system trust store must be world-readable (0644); no private key is written here. fmt.Fprintf(os.Stderr, "Warning: could not install CA certificate: %v\n", err) return } @@ -3757,7 +3762,7 @@ func runAdminBanner(cmd *cobra.Command, _ []string) { // Write to /etc/issue — displayed on the console before the login prompt. // Overwrite entirely; this is a purpose-built appliance so we own this file. - if err := os.WriteFile("/etc/issue", []byte(banner), 0o644); err != nil { + if err := os.WriteFile("/etc/issue", []byte(banner), 0o644); err != nil { //nolint:gosec // G306: /etc/issue is shown before login and must be world-readable (0644). Contains no secret material. slog.Warn("Failed to write /etc/issue", "err", err) } @@ -3791,7 +3796,7 @@ func appendBannerToMotd(banner string) error { base += "\n\n" } } - return os.WriteFile(motdPath, []byte(base+sentinel+banner), 0o644) + return os.WriteFile(motdPath, []byte(base+sentinel+banner), 0o644) //nolint:gosec // G306: motd is shown at login and must be world-readable (0644). Contains no secret material. } // gpuBannerSection returns an optional banner box section describing GPU state. @@ -3913,7 +3918,7 @@ func writeNodeConf(path string, conf map[string]string) error { fmt.Fprintf(&b, "%s=%s\n", k, v) } } - return os.WriteFile(path, []byte(b.String()), 0o644) + return os.WriteFile(path, []byte(b.String()), 0o644) //nolint:gosec // G306: node.conf holds the management IP, interface and hostname, read by non-root Spinifex services (0644). Contains no secret material. } // resolveIfaceIP returns the first IPv4 address assigned to iface, or "". diff --git a/cmd/spinifex/cmd/admin_account_delete.go b/spinifex/operator/cli/admin_account_delete.go similarity index 99% rename from cmd/spinifex/cmd/admin_account_delete.go rename to spinifex/operator/cli/admin_account_delete.go index 806a93faa..6ddf9a4fc 100644 --- a/cmd/spinifex/cmd/admin_account_delete.go +++ b/spinifex/operator/cli/admin_account_delete.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bufio" diff --git a/cmd/spinifex/cmd/admin_account_delete_output_test.go b/spinifex/operator/cli/admin_account_delete_output_test.go similarity index 92% rename from cmd/spinifex/cmd/admin_account_delete_output_test.go rename to spinifex/operator/cli/admin_account_delete_output_test.go index af7ea0db5..d5740b067 100644 --- a/cmd/spinifex/cmd/admin_account_delete_output_test.go +++ b/spinifex/operator/cli/admin_account_delete_output_test.go @@ -1,4 +1,4 @@ -package cmd_test +package cli_test import ( "bytes" @@ -6,8 +6,8 @@ import ( "os" "testing" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" "github.com/mulgadc/spinifex/spinifex/accountteardown" + "github.com/mulgadc/spinifex/spinifex/operator/cli" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -75,7 +75,7 @@ func teardownPlan() *accountteardown.Result { // The plan is what the operator confirms against, so every resource has to // appear under its stage — and an empty stage must not pad the list. func TestPrintTeardownPlanListsEveryResource(t *testing.T) { - output := captureOutput(t, func() { cmd.PrintTeardownPlan(teardownPlan()) }) + output := captureOutput(t, func() { cli.PrintTeardownPlan(teardownPlan()) }) assert.Contains(t, output, "000000000042") assert.Contains(t, output, "tenant@example.com") @@ -90,7 +90,7 @@ func TestPrintTeardownPlanListsEveryResource(t *testing.T) { // output, which is exactly when an operator stops and asks. func TestPrintTeardownPlanSaysWhenTheAccountIsEmpty(t *testing.T) { output := captureOutput(t, func() { - cmd.PrintTeardownPlan(&accountteardown.Result{ + cli.PrintTeardownPlan(&accountteardown.Result{ AccountID: "000000000042", AccountName: "tenant@example.com", Stages: []accountteardown.StageResult{{Stage: accountteardown.StageCompute, Elapsed: "0s"}}, @@ -123,7 +123,7 @@ func TestPrintTeardownResultNamesStuckResources(t *testing.T) { }, } - output := captureOutput(t, func() { cmd.PrintTeardownResult(result) }) + output := captureOutput(t, func() { cli.PrintTeardownResult(result) }) assert.Contains(t, output, "1 deleted, 0 stuck") assert.Contains(t, output, "STUCK") @@ -140,7 +140,7 @@ func TestPromptAccountNameReadsTheConfirmation(t *testing.T) { captureOutput(t, func() { feedStdin(t, " tenant@example.com \n", func() { - name, err = cmd.PromptAccountName("000000000042") + name, err = cli.PromptAccountName("000000000042") }) }) @@ -153,7 +153,7 @@ func TestPromptAccountNameTreatsAnEmptyReplyAsCancelled(t *testing.T) { captureOutput(t, func() { feedStdin(t, "\n", func() { - _, err = cmd.PromptAccountName("000000000042") + _, err = cli.PromptAccountName("000000000042") }) }) diff --git a/cmd/spinifex/cmd/admin_account_delete_remote.go b/spinifex/operator/cli/admin_account_delete_remote.go similarity index 99% rename from cmd/spinifex/cmd/admin_account_delete_remote.go rename to spinifex/operator/cli/admin_account_delete_remote.go index 1290c2eab..6620e0759 100644 --- a/cmd/spinifex/cmd/admin_account_delete_remote.go +++ b/spinifex/operator/cli/admin_account_delete_remote.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_account_delete_remote_test.go b/spinifex/operator/cli/admin_account_delete_remote_test.go similarity index 82% rename from cmd/spinifex/cmd/admin_account_delete_remote_test.go rename to spinifex/operator/cli/admin_account_delete_remote_test.go index 9f89a3483..f8f126c7e 100644 --- a/cmd/spinifex/cmd/admin_account_delete_remote_test.go +++ b/spinifex/operator/cli/admin_account_delete_remote_test.go @@ -1,4 +1,4 @@ -package cmd_test +package cli_test import ( "context" @@ -9,8 +9,8 @@ import ( "testing" "time" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" "github.com/mulgadc/spinifex/spinifex/gateway" + "github.com/mulgadc/spinifex/spinifex/operator/cli" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -51,8 +51,8 @@ func TestDeleteAccountRemoteSignsAndPosts(t *testing.T) { srv := newAdminTestServer(t, http.StatusOK, `{"deletionId":"d-1","accountId":"000000000042","state":"RUNNING"}`, &got) - out, err := cmd.DeleteAccountRemote(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), + out, err := cli.DeleteAccountRemote(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), gateway.DeleteAccountRequest{ AccountID: "000000000042", AccountName: "tenant@example.com", @@ -78,8 +78,8 @@ func TestDeleteAccountRemoteDryRunCarriesTheInventory(t *testing.T) { `{"accountId":"000000000042","state":"DRY_RUN","dryRun":true,"inventory":{"accountId":"000000000042",`+ `"dryRun":true,"stages":[{"stage":1,"deleted":[{"kind":"instance","id":"i-1"}],"elapsed":"0s"}]}}`, &got) - out, err := cmd.DeleteAccountRemote(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), + out, err := cli.DeleteAccountRemote(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), gateway.DeleteAccountRequest{AccountID: "000000000042", DryRun: true}) require.NoError(t, err) @@ -97,8 +97,8 @@ func TestDeleteAccountRemoteSurfacesGatewayError(t *testing.T) { srv := newAdminTestServer(t, http.StatusForbidden, `{"error":{"code":"AccessDenied","message":"denied"},"requestId":"req-9"}`, &got) - _, err := cmd.DeleteAccountRemote(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), + _, err := cli.DeleteAccountRemote(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), gateway.DeleteAccountRequest{ AccountID: "000000000001", AccountName: "spinifex", @@ -108,7 +108,7 @@ func TestDeleteAccountRemoteSurfacesGatewayError(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "AccessDenied") assert.Contains(t, err.Error(), "req-9") - assert.False(t, cmd.RetryableAdminError(err), "a protected account is not fixed by retrying") + assert.False(t, cli.RetryableAdminError(err), "a protected account is not fixed by retrying") } func TestDescribeAccountDeletionRemoteReportsProgress(t *testing.T) { @@ -118,8 +118,8 @@ func TestDescribeAccountDeletionRemoteReportsProgress(t *testing.T) { srv := newAdminTestServer(t, http.StatusOK, `{"deletionId":"d-1","state":"RUNNING","stages":[{"stage":1,"elapsed":"5s"}]}`, &got) - out, err := cmd.DescribeAccountDeletionRemote(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") + out, err := cli.DescribeAccountDeletionRemote(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") require.NoError(t, err) assert.Equal(t, "/admin/DescribeAccountDeletion", got.URL.Path) @@ -132,7 +132,7 @@ func TestDescribeAccountDeletionRemoteReportsProgress(t *testing.T) { // terminal state. func TestFollowAccountDeletionPollsUntilComplete(t *testing.T) { adminTestCredentials(t) - defer cmd.SetAccountDeletePollInterval(time.Millisecond)() + defer cli.SetAccountDeletePollInterval(time.Millisecond)() srv, calls := newScriptedAdminServer(t, `{"deletionId":"d-1","state":"RUNNING","stages":[{"stage":1,"elapsed":"5s"}]}`, @@ -140,8 +140,8 @@ func TestFollowAccountDeletionPollsUntilComplete(t *testing.T) { `{"deletionId":"d-1","state":"COMPLETED","stages":[{"stage":1,"elapsed":"5s"},{"stage":2,"elapsed":"1s"}]}`, ) - err := cmd.FollowAccountDeletion(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") + err := cli.FollowAccountDeletion(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") require.NoError(t, err) assert.Equal(t, int32(3), calls.Load(), "the loop must keep polling while the job runs") @@ -151,13 +151,13 @@ func TestFollowAccountDeletionPollsUntilComplete(t *testing.T) { // left TERMINATING, and an operator who reads "done" would not go looking. func TestFollowAccountDeletionReportsFailure(t *testing.T) { adminTestCredentials(t) - defer cmd.SetAccountDeletePollInterval(time.Millisecond)() + defer cli.SetAccountDeletePollInterval(time.Millisecond)() srv, _ := newScriptedAdminServer(t, `{"deletionId":"d-1","state":"FAILED","stages":[],"error":"2 resources left"}`) - err := cmd.FollowAccountDeletion(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") + err := cli.FollowAccountDeletion(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") require.Error(t, err) assert.Contains(t, err.Error(), "2 resources left") @@ -167,15 +167,15 @@ func TestFollowAccountDeletionReportsFailure(t *testing.T) { // message has to name the account the caller must go back to. func TestFollowAccountDeletionStopsWhenTheContextEnds(t *testing.T) { adminTestCredentials(t) - defer cmd.SetAccountDeletePollInterval(50 * time.Millisecond)() + defer cli.SetAccountDeletePollInterval(50 * time.Millisecond)() srv, _ := newScriptedAdminServer(t, `{"deletionId":"d-1","state":"RUNNING","stages":[]}`) ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) defer cancel() - err := cmd.FollowAccountDeletion(ctx, - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") + err := cli.FollowAccountDeletion(ctx, + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") require.Error(t, err) assert.Contains(t, err.Error(), "000000000042") @@ -185,14 +185,14 @@ func TestFollowAccountDeletionStopsWhenTheContextEnds(t *testing.T) { // a teardown still holding an account's resources. func TestFollowAccountDeletionFailsOnAnUnreadableJob(t *testing.T) { adminTestCredentials(t) - defer cmd.SetAccountDeletePollInterval(time.Millisecond)() + defer cli.SetAccountDeletePollInterval(time.Millisecond)() var got http.Request srv := newAdminTestServer(t, http.StatusNotFound, `{"error":{"code":"NoSuchEntity","message":"no job"},"requestId":"req-2"}`, &got) - err := cmd.FollowAccountDeletion(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") + err := cli.FollowAccountDeletion(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), "000000000042") require.Error(t, err) assert.Contains(t, err.Error(), "NoSuchEntity") diff --git a/cmd/spinifex/cmd/admin_account_list_remote.go b/spinifex/operator/cli/admin_account_list_remote.go similarity index 99% rename from cmd/spinifex/cmd/admin_account_list_remote.go rename to spinifex/operator/cli/admin_account_list_remote.go index 52ea5cee8..66f256f2b 100644 --- a/cmd/spinifex/cmd/admin_account_list_remote.go +++ b/spinifex/operator/cli/admin_account_list_remote.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_account_quota.go b/spinifex/operator/cli/admin_account_quota.go similarity index 99% rename from cmd/spinifex/cmd/admin_account_quota.go rename to spinifex/operator/cli/admin_account_quota.go index 0d1f8f923..787f4d5ca 100644 --- a/cmd/spinifex/cmd/admin_account_quota.go +++ b/spinifex/operator/cli/admin_account_quota.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_account_quota_test.go b/spinifex/operator/cli/admin_account_quota_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_account_quota_test.go rename to spinifex/operator/cli/admin_account_quota_test.go index 76f188368..a0ef0268a 100644 --- a/cmd/spinifex/cmd/admin_account_quota_test.go +++ b/spinifex/operator/cli/admin_account_quota_test.go @@ -1,5 +1,5 @@ //test:in-package — drives the unexported flag table, parser and printer the quota subcommands are built from. -package cmd +package cli import ( "testing" diff --git a/cmd/spinifex/cmd/admin_acm.go b/spinifex/operator/cli/admin_acm.go similarity index 98% rename from cmd/spinifex/cmd/admin_acm.go rename to spinifex/operator/cli/admin_acm.go index eae8d41a6..5c9cbb348 100644 --- a/cmd/spinifex/cmd/admin_acm.go +++ b/spinifex/operator/cli/admin_acm.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bufio" @@ -60,12 +60,12 @@ func init() { certCreateTenantCACmd.Flags().StringSlice("domain", nil, "Domain the tenant CA is permitted to sign for (repeatable)") certCreateTenantCACmd.Flags().Bool("regenerate", false, "Replace an existing tenant CA with a fresh root (invalidates every device's trust store — requires confirmation)") certCreateTenantCACmd.Flags().Bool("yes", false, "Skip the --regenerate confirmation prompt") - _ = certCreateTenantCACmd.MarkFlagRequired("domain") + mustMarkFlagRequired(certCreateTenantCACmd, "domain") certCmd.AddCommand(certForceRenewCmd) certForceRenewCmd.Flags().String("arn", "", "Certificate ARN to force-renew (required)") certForceRenewCmd.Flags().String("account", "", "Account ID that owns the certificate; defaults to the bootstrap account") - _ = certForceRenewCmd.MarkFlagRequired("arn") + mustMarkFlagRequired(certForceRenewCmd, "arn") } func runCertForceRenew(cmd *cobra.Command, _ []string) { diff --git a/cmd/spinifex/cmd/admin_eks.go b/spinifex/operator/cli/admin_eks.go similarity index 97% rename from cmd/spinifex/cmd/admin_eks.go rename to spinifex/operator/cli/admin_eks.go index 5320f2c73..cd2e5ffd6 100644 --- a/cmd/spinifex/cmd/admin_eks.go +++ b/spinifex/operator/cli/admin_eks.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" @@ -38,7 +38,7 @@ func init() { adminEksRestoreSnapshotCmd.Flags().String("cluster", "", "Cluster name to restore (required)") adminEksRestoreSnapshotCmd.Flags().String("snapshot", "", "Etcd snapshot key to restore; defaults to the latest snapshot found in predastore") adminEksRestoreSnapshotCmd.Flags().String("account", "", "Account ID that owns the cluster; defaults to the bootstrap account") - _ = adminEksRestoreSnapshotCmd.MarkFlagRequired("cluster") + mustMarkFlagRequired(adminEksRestoreSnapshotCmd, "cluster") } func runAdminEksRestoreSnapshot(cmd *cobra.Command, _ []string) { diff --git a/cmd/spinifex/cmd/admin_firewall.go b/spinifex/operator/cli/admin_firewall.go similarity index 99% rename from cmd/spinifex/cmd/admin_firewall.go rename to spinifex/operator/cli/admin_firewall.go index f68769c08..027194ba0 100644 --- a/cmd/spinifex/cmd/admin_firewall.go +++ b/spinifex/operator/cli/admin_firewall.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "fmt" diff --git a/cmd/spinifex/cmd/admin_firewall_test.go b/spinifex/operator/cli/admin_firewall_test.go similarity index 86% rename from cmd/spinifex/cmd/admin_firewall_test.go rename to spinifex/operator/cli/admin_firewall_test.go index 2efb46016..9d504c0b1 100644 --- a/cmd/spinifex/cmd/admin_firewall_test.go +++ b/spinifex/operator/cli/admin_firewall_test.go @@ -1,4 +1,4 @@ -package cmd_test +package cli_test import ( "os" @@ -6,7 +6,7 @@ import ( "strings" "testing" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" + "github.com/mulgadc/spinifex/spinifex/operator/cli" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -20,7 +20,7 @@ func stubFirewallHelper(t *testing.T, body string) string { helper := filepath.Join(dir, "spinifex-firewall-apply") script := "#!/bin/sh\necho \"$*\" >> " + log + "\n" + body require.NoError(t, os.WriteFile(helper, []byte(script), 0o755)) - t.Cleanup(cmd.SetFirewallApplyHelper(helper)) + t.Cleanup(cli.SetFirewallApplyHelper(helper)) return log } @@ -37,7 +37,7 @@ func helperCalls(t *testing.T, log string) []string { func TestOpenFormationPort(t *testing.T) { log := stubFirewallHelper(t, "exit 0\n") - closePort := cmd.OpenFormationPort(4432) + closePort := cli.OpenFormationPort(4432) assert.Equal(t, []string{"open-port", "4432"}, helperCalls(t, log)) closePort() @@ -48,9 +48,9 @@ func TestOpenFormationPort(t *testing.T) { // A machine with no policy installed has nothing to open, and formation must not // be held up by looking for a helper that was never installed. func TestOpenFormationPort_NoHelperInstalled(t *testing.T) { - t.Cleanup(cmd.SetFirewallApplyHelper(filepath.Join(t.TempDir(), "absent"))) + t.Cleanup(cli.SetFirewallApplyHelper(filepath.Join(t.TempDir(), "absent"))) - closePort := cmd.OpenFormationPort(4432) + closePort := cli.OpenFormationPort(4432) require.NotNil(t, closePort) closePort() } @@ -61,7 +61,7 @@ func TestOpenFormationPort_NoHelperInstalled(t *testing.T) { func TestOpenFormationPort_FailureDoesNotBlockFormation(t *testing.T) { log := stubFirewallHelper(t, "exit 1\n") - closePort := cmd.OpenFormationPort(4432) + closePort := cli.OpenFormationPort(4432) require.NotNil(t, closePort) closePort() diff --git a/cmd/spinifex/cmd/admin_gpu.go b/spinifex/operator/cli/admin_gpu.go similarity index 96% rename from cmd/spinifex/cmd/admin_gpu.go rename to spinifex/operator/cli/admin_gpu.go index 44d64f904..a2e752287 100644 --- a/cmd/spinifex/cmd/admin_gpu.go +++ b/spinifex/operator/cli/admin_gpu.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "encoding/json" @@ -329,7 +329,7 @@ func runAdminGpuSetup(_ *cobra.Command, _ []string) { const vfioUdevRule = "/etc/udev/rules.d/99-spinifex-vfio.rules" if _, err := os.Stat(vfioUdevRule); os.IsNotExist(err) { rule := "SUBSYSTEM==\"vfio\", GROUP=\"spinifex\", MODE=\"0660\"\n" - if err := os.WriteFile(vfioUdevRule, []byte(rule), 0o644); err != nil { + if err := os.WriteFile(vfioUdevRule, []byte(rule), 0o644); err != nil { //nolint:gosec // G306: udev rule granting the spinifex group VFIO access is conventionally world-readable (0644). Contains no secret material. fmt.Fprintf(os.Stderr, "Failed to write vfio udev rule: %v\n", err) os.Exit(1) } @@ -343,7 +343,7 @@ func runAdminGpuSetup(_ *cobra.Command, _ []string) { fmt.Println("==> Configuring nouveau blacklist") const nouveauConf = "/etc/modprobe.d/blacklist-nouveau.conf" if _, err := os.Stat(nouveauConf); os.IsNotExist(err) { - if err := os.WriteFile(nouveauConf, []byte("blacklist nouveau\noptions nouveau modeset=0\n"), 0o644); err != nil { + if err := os.WriteFile(nouveauConf, []byte("blacklist nouveau\noptions nouveau modeset=0\n"), 0o644); err != nil { //nolint:gosec // G306: modprobe.d blacklist is conventionally world-readable (0644). Contains no secret material. fmt.Fprintf(os.Stderr, "Failed to write nouveau blacklist: %v\n", err) os.Exit(1) } @@ -364,7 +364,7 @@ func runAdminGpuSetup(_ *cobra.Command, _ []string) { } if hasAMD { if _, err := os.Stat(amdgpuConf); os.IsNotExist(err) { - if err := os.WriteFile(amdgpuConf, []byte("blacklist amdgpu\n"), 0o644); err != nil { + if err := os.WriteFile(amdgpuConf, []byte("blacklist amdgpu\n"), 0o644); err != nil { //nolint:gosec // G306: modprobe.d blacklist is conventionally world-readable (0644). Contains no secret material. fmt.Fprintf(os.Stderr, "Failed to write amdgpu blacklist: %v\n", err) os.Exit(1) } @@ -381,7 +381,7 @@ func runAdminGpuSetup(_ *cobra.Command, _ []string) { const vfioPCIConf = "/etc/modprobe.d/vfio-pci.conf" existing, _ := os.ReadFile(vfioPCIConf) if !strings.Contains(string(existing), "ids="+idsCSV) { - if err := os.WriteFile(vfioPCIConf, []byte("options vfio-pci ids="+idsCSV+"\n"), 0o644); err != nil { + if err := os.WriteFile(vfioPCIConf, []byte("options vfio-pci ids="+idsCSV+"\n"), 0o644); err != nil { //nolint:gosec // G306: modprobe.d vfio-pci options are conventionally world-readable (0644). Contains no secret material. fmt.Fprintf(os.Stderr, "Failed to write vfio-pci.conf: %v\n", err) os.Exit(1) } @@ -394,7 +394,7 @@ func runAdminGpuSetup(_ *cobra.Command, _ []string) { const initramfsModules = "/etc/initramfs-tools/modules" modData, _ := os.ReadFile(initramfsModules) if !strings.Contains(string(modData), "vfio_pci") { - f, err := os.OpenFile(initramfsModules, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o644) + f, err := os.OpenFile(initramfsModules, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o644) //nolint:gosec // G302: initramfs modules list is conventionally world-readable (0644). Contains no secret material. if err != nil { fmt.Fprintf(os.Stderr, "Failed to open initramfs modules: %v\n", err) os.Exit(1) @@ -796,7 +796,7 @@ func gpuSetupAddGRUBParams(params string) (bool, error) { if updated == content { return false, fmt.Errorf("GRUB_CMDLINE_LINUX_DEFAULT not found in %s", grubFile) } - if err := os.WriteFile(grubFile, []byte(updated), 0o644); err != nil { + if err := os.WriteFile(grubFile, []byte(updated), 0o644); err != nil { //nolint:gosec // G306: /etc/default/grub is read by grub-mkconfig and is conventionally 0644. Contains no secret material. return false, fmt.Errorf("write %s: %w", grubFile, err) } return true, nil diff --git a/cmd/spinifex/cmd/admin_jsprobe.go b/spinifex/operator/cli/admin_jsprobe.go similarity index 99% rename from cmd/spinifex/cmd/admin_jsprobe.go rename to spinifex/operator/cli/admin_jsprobe.go index 558b2ab41..835883150 100644 --- a/cmd/spinifex/cmd/admin_jsprobe.go +++ b/spinifex/operator/cli/admin_jsprobe.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_jsprobe_test.go b/spinifex/operator/cli/admin_jsprobe_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_jsprobe_test.go rename to spinifex/operator/cli/admin_jsprobe_test.go index d5d926ee3..94aaa689a 100644 --- a/cmd/spinifex/cmd/admin_jsprobe_test.go +++ b/spinifex/operator/cli/admin_jsprobe_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_kv.go b/spinifex/operator/cli/admin_kv.go similarity index 99% rename from cmd/spinifex/cmd/admin_kv.go rename to spinifex/operator/cli/admin_kv.go index bc5baf73c..ef4748668 100644 --- a/cmd/spinifex/cmd/admin_kv.go +++ b/spinifex/operator/cli/admin_kv.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/admin_kv_compare.go b/spinifex/operator/cli/admin_kv_compare.go similarity index 99% rename from cmd/spinifex/cmd/admin_kv_compare.go rename to spinifex/operator/cli/admin_kv_compare.go index db47f8e3f..97cb0517e 100644 --- a/cmd/spinifex/cmd/admin_kv_compare.go +++ b/spinifex/operator/cli/admin_kv_compare.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "encoding/json" diff --git a/cmd/spinifex/cmd/admin_kv_compare_test.go b/spinifex/operator/cli/admin_kv_compare_test.go similarity index 83% rename from cmd/spinifex/cmd/admin_kv_compare_test.go rename to spinifex/operator/cli/admin_kv_compare_test.go index 052daf011..a9031f9d2 100644 --- a/cmd/spinifex/cmd/admin_kv_compare_test.go +++ b/spinifex/operator/cli/admin_kv_compare_test.go @@ -1,4 +1,4 @@ -package cmd_test +package cli_test import ( "encoding/json" @@ -6,7 +6,7 @@ import ( "testing" "time" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" + "github.com/mulgadc/spinifex/spinifex/operator/cli" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -80,7 +80,7 @@ func kvs(msgs ...msg) []msg { return msgs } func TestKVCompareIdentical(t *testing.T) { s := stream{name: "KV_a", replicas: 3, msgs: kvs(msg{1, "k", "h1", t0}, msg{2, "k", "h2", t0})} // The digest field embeds the host, so this exercises the per-seq path too. - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, s), doc(t, "n2", t0, s), doc(t, "n3", t0, s)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, s), doc(t, "n2", t0, s), doc(t, "n3", t0, s)) require.NoError(t, err) assert.False(t, divergent, report) assert.Contains(t, report, "1 streams: 1 consistent, 0 diverged") @@ -91,7 +91,7 @@ func TestKVCompareIdentical(t *testing.T) { func TestKVCompareSameSeqDifferentContent(t *testing.T) { a := stream{name: "KV_acct", replicas: 2, msgs: kvs(msg{1, "$KV.acct.000000000002", "aaaa", t0})} b := stream{name: "KV_acct", replicas: 2, msgs: kvs(msg{1, "$KV.acct.000000000002", "bbbb", t0})} - divergent, report, err := cmd.CompareKVDigests(doc(t, "node1", t0, a), doc(t, "node2", t0, b)) + divergent, report, err := cli.CompareKVDigests(doc(t, "node1", t0, a), doc(t, "node2", t0, b)) require.NoError(t, err) assert.True(t, divergent) assert.Contains(t, report, "DIVERGED KV_acct") @@ -104,7 +104,7 @@ func TestKVCompareMissingMessage(t *testing.T) { t.Run("superseded by a later write to the subject", func(t *testing.T) { lagging := stream{name: "KV_a", replicas: 2, msgs: full} purged := stream{name: "KV_a", replicas: 2, msgs: kvs(msg{1, "", "deleted", time.Time{}}, full[1], full[2])} - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, lagging), doc(t, "n2", t0, purged)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, lagging), doc(t, "n2", t0, purged)) require.NoError(t, err) assert.False(t, divergent, report) assert.Contains(t, report, "1 superseded or expired") @@ -113,7 +113,7 @@ func TestKVCompareMissingMessage(t *testing.T) { t.Run("nothing explains it", func(t *testing.T) { have := stream{name: "KV_a", replicas: 2, msgs: full} lost := stream{name: "KV_a", replicas: 2, msgs: kvs(full[0], msg{2, "", "deleted", time.Time{}}, full[2])} - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, have), doc(t, "n2", t0, lost)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, have), doc(t, "n2", t0, lost)) require.NoError(t, err) assert.True(t, divergent) assert.Contains(t, report, "seq 2 j is on n1 but missing from n2") @@ -123,12 +123,12 @@ func TestKVCompareMissingMessage(t *testing.T) { old := t0.Add(-time.Hour) have := stream{name: "KV_lease", replicas: 2, maxAge: time.Minute, msgs: kvs(msg{1, "l", "h1", old}, msg{2, "m", "h2", t0})} gone := stream{name: "KV_lease", replicas: 2, maxAge: time.Minute, msgs: kvs(msg{2, "m", "h2", t0})} - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, have), doc(t, "n2", t0, gone)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, have), doc(t, "n2", t0, gone)) require.NoError(t, err) assert.False(t, divergent, report) have.maxAge, gone.maxAge = 0, 0 - divergent, _, err = cmd.CompareKVDigests(doc(t, "n1", t0, have), doc(t, "n2", t0, gone)) + divergent, _, err = cli.CompareKVDigests(doc(t, "n1", t0, have), doc(t, "n2", t0, gone)) require.NoError(t, err) assert.True(t, divergent, "without max_age the same gap is unexplained") }) @@ -136,7 +136,7 @@ func TestKVCompareMissingMessage(t *testing.T) { t.Run("head dropped by a stream limit", func(t *testing.T) { behind := stream{name: "S", replicas: 2, maxMsgs: 2, msgs: kvs(msg{1, "a", "h1", t0}, msg{2, "b", "h2", t0})} ahead := stream{name: "S", replicas: 2, maxMsgs: 2, msgs: kvs(msg{2, "b", "h2", t0}, msg{3, "c", "h3", t0})} - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, behind), doc(t, "n2", t0, ahead)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, behind), doc(t, "n2", t0, ahead)) require.NoError(t, err) assert.False(t, divergent, report) }) @@ -145,7 +145,7 @@ func TestKVCompareMissingMessage(t *testing.T) { func TestKVCompareTailLag(t *testing.T) { behind := stream{name: "KV_a", replicas: 2, msgs: kvs(msg{1, "k", "h1", t0}, msg{2, "k", "h2", t0})} ahead := stream{name: "KV_a", replicas: 2, msgs: kvs(msg{1, "k", "h1", t0}, msg{2, "k", "h2", t0}, msg{3, "k", "h3", t0}, msg{4, "k", "h4", t0})} - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, behind), doc(t, "n2", t0, ahead)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, behind), doc(t, "n2", t0, ahead)) require.NoError(t, err) assert.False(t, divergent, report) assert.Contains(t, report, "tail not compared: n2 +2") @@ -154,7 +154,7 @@ func TestKVCompareTailLag(t *testing.T) { func TestKVCompareNoCommonRange(t *testing.T) { a := stream{name: "KV_a", replicas: 2, msgs: kvs(msg{1, "k", "h1", t0})} b := stream{name: "KV_a", replicas: 2, msgs: kvs(msg{5, "k", "h5", t0})} - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, a), doc(t, "n2", t0, b)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, a), doc(t, "n2", t0, b)) require.NoError(t, err) assert.True(t, divergent) assert.Contains(t, report, "no sequence range is common") @@ -165,14 +165,14 @@ func TestKVCompareReplicaPlacement(t *testing.T) { other := stream{name: "KV_b", replicas: 1, msgs: kvs(msg{1, "k", "h1", t0})} t.Run("fewer holders than replicas", func(t *testing.T) { - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, s), doc(t, "n2", t0, s), doc(t, "n3", t0, other)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, s), doc(t, "n2", t0, s), doc(t, "n3", t0, other)) require.NoError(t, err) assert.True(t, divergent) assert.Contains(t, report, "held by 2 of 3 replicas; missing from n3") }) t.Run("more holders than replicas", func(t *testing.T) { - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, other), doc(t, "n2", t0, other)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, other), doc(t, "n2", t0, other)) require.NoError(t, err) assert.True(t, divergent) assert.Contains(t, report, "held by 2 nodes but configured for 1 replicas") @@ -181,14 +181,14 @@ func TestKVCompareReplicaPlacement(t *testing.T) { t.Run("replica counts disagree", func(t *testing.T) { r1 := s r1.replicas = 1 - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, s), doc(t, "n2", t0, r1)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, s), doc(t, "n2", t0, r1)) require.NoError(t, err) assert.True(t, divergent) assert.Contains(t, report, "replicas disagree on the replica count") }) t.Run("single replica stream on one node", func(t *testing.T) { - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, other), doc(t, "n2", t0)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, other), doc(t, "n2", t0)) require.NoError(t, err) assert.False(t, divergent, report) }) @@ -197,7 +197,7 @@ func TestKVCompareReplicaPlacement(t *testing.T) { func TestKVCompareNeedsSeqs(t *testing.T) { a := stream{name: "KV_a", replicas: 2, noSeqs: true, msgs: kvs(msg{1, "k", "h1", t0})} b := stream{name: "KV_a", replicas: 2, noSeqs: true, msgs: kvs(msg{1, "k", "h2", t0})} - divergent, report, err := cmd.CompareKVDigests(doc(t, "n1", t0, a), doc(t, "n2", t0, b)) + divergent, report, err := cli.CompareKVDigests(doc(t, "n1", t0, a), doc(t, "n2", t0, b)) require.NoError(t, err) assert.True(t, divergent) assert.Contains(t, report, "re-run digest with --seqs") @@ -207,27 +207,27 @@ func TestKVCompareInput(t *testing.T) { s := stream{name: "KV_a", replicas: 2, msgs: kvs(msg{1, "k", "h1", t0})} // Several documents in one stream, as "-" reads them from stdin. - divergent, _, err := cmd.CompareKVDigests(doc(t, "n1", t0, s) + "\n" + doc(t, "n2", t0, s)) + divergent, _, err := cli.CompareKVDigests(doc(t, "n1", t0, s) + "\n" + doc(t, "n2", t0, s)) require.NoError(t, err) assert.False(t, divergent) - _, _, err = cmd.CompareKVDigests(doc(t, "n1", t0, s)) + _, _, err = cli.CompareKVDigests(doc(t, "n1", t0, s)) assert.ErrorContains(t, err, "at least two nodes") - _, _, err = cmd.CompareKVDigests(doc(t, "n1", t0, s), doc(t, "n1", t0, s)) + _, _, err = cli.CompareKVDigests(doc(t, "n1", t0, s), doc(t, "n1", t0, s)) assert.ErrorContains(t, err, "two digests are from host n1") - _, _, err = cmd.CompareKVDigests(`KV_a first=1 last=1`) + _, _, err = cli.CompareKVDigests(`KV_a first=1 last=1`) assert.Error(t, err, "text digest output is rejected") - _, _, err = cmd.CompareKVDigests(`{"streams":[]}`) + _, _, err = cli.CompareKVDigests(`{"streams":[]}`) assert.ErrorContains(t, err, "no host") - _, _, err = cmd.CompareKVDigests(" ") + _, _, err = cli.CompareKVDigests(" ") assert.ErrorContains(t, err, "no digest documents") } func TestAdminInitDiscardsJetStreamByDefault(t *testing.T) { - assert.Equal(t, "true", cmd.AdminInitFlagDefault("discard-jetstream")) - assert.True(t, strings.HasPrefix(cmd.AdminInitFlagDefault("ipsec"), "true")) + assert.Equal(t, "true", cli.AdminInitFlagDefault("discard-jetstream")) + assert.True(t, strings.HasPrefix(cli.AdminInitFlagDefault("ipsec"), "true")) } diff --git a/cmd/spinifex/cmd/admin_kv_replicas.go b/spinifex/operator/cli/admin_kv_replicas.go similarity index 99% rename from cmd/spinifex/cmd/admin_kv_replicas.go rename to spinifex/operator/cli/admin_kv_replicas.go index ece5ec5dc..919ed1ccf 100644 --- a/cmd/spinifex/cmd/admin_kv_replicas.go +++ b/spinifex/operator/cli/admin_kv_replicas.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_kv_replicas_test.go b/spinifex/operator/cli/admin_kv_replicas_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_kv_replicas_test.go rename to spinifex/operator/cli/admin_kv_replicas_test.go index 499ff5e0d..65c601c88 100644 --- a/cmd/spinifex/cmd/admin_kv_replicas_test.go +++ b/spinifex/operator/cli/admin_kv_replicas_test.go @@ -1,7 +1,7 @@ //test:in-package — it tests this command's unexported rendering and repair // helpers, which exist only to be exercised without a live cluster. -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/admin_kv_test.go b/spinifex/operator/cli/admin_kv_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_kv_test.go rename to spinifex/operator/cli/admin_kv_test.go index 5197ea4c6..8ea2b02ed 100644 --- a/cmd/spinifex/cmd/admin_kv_test.go +++ b/spinifex/operator/cli/admin_kv_test.go @@ -1,7 +1,7 @@ //test:in-package: exercises the unexported digest helpers against a real clustered // store, which the cobra command only reaches through a live node. -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/admin_nat_test.go b/spinifex/operator/cli/admin_nat_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_nat_test.go rename to spinifex/operator/cli/admin_nat_test.go index 1f536d83a..8cf540000 100644 --- a/cmd/spinifex/cmd/admin_nat_test.go +++ b/spinifex/operator/cli/admin_nat_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "os" diff --git a/cmd/spinifex/cmd/admin_node_count_test.go b/spinifex/operator/cli/admin_node_count_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_node_count_test.go rename to spinifex/operator/cli/admin_node_count_test.go index 2b1ca3f71..3d5e212d8 100644 --- a/cmd/spinifex/cmd/admin_node_count_test.go +++ b/spinifex/operator/cli/admin_node_count_test.go @@ -1,7 +1,7 @@ //test:in-package — validateNodeCount is an unexported guard on this command's // flags, and exporting it would widen the package's API for a test's convenience. -package cmd +package cli import ( "strings" diff --git a/cmd/spinifex/cmd/admin_ochre.go b/spinifex/operator/cli/admin_ochre.go similarity index 99% rename from cmd/spinifex/cmd/admin_ochre.go rename to spinifex/operator/cli/admin_ochre.go index ca604b045..70d47aa3e 100644 --- a/cmd/spinifex/cmd/admin_ochre.go +++ b/spinifex/operator/cli/admin_ochre.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bytes" @@ -128,24 +128,24 @@ func init() { ochreWeightsStageCmd.Flags().String("model-id", "", "Catalog model ID to stage weights for (required)") ochreWeightsStageCmd.Flags().String("s3-uri", "", "predastore S3 URI holding the model's Hugging Face files, e.g. s3://bucket/prefix/ (required)") ochreWeightsStageCmd.Flags().String("tmp-dir", os.TempDir(), "Temporary directory for download and volume staging") - _ = ochreWeightsStageCmd.MarkFlagRequired("model-id") - _ = ochreWeightsStageCmd.MarkFlagRequired("s3-uri") + mustMarkFlagRequired(ochreWeightsStageCmd, "model-id") + mustMarkFlagRequired(ochreWeightsStageCmd, "s3-uri") ochreWeightsRemoveCmd.Flags().String("model-id", "", "Model ID to remove from staged weights (required)") - _ = ochreWeightsRemoveCmd.MarkFlagRequired("model-id") + mustMarkFlagRequired(ochreWeightsRemoveCmd, "model-id") ochreWeightsPullCmd.Flags().String("model-id", "", "Self-host catalog model ID this pull is for (required)") ochreWeightsPullCmd.Flags().String("hf-repo", "", "Hugging Face repo, e.g. meta-llama/Llama-3.2-1B-Instruct (default: the catalog entry's repo)") ochreWeightsPullCmd.Flags().String("revision", "main", "Hugging Face branch, tag or commit SHA to resolve and pin (default: the catalog entry's pinned revision)") ochreWeightsPullCmd.Flags().String("s3-uri", "", "predastore destination prefix, e.g. s3://bucket/prefix/ (default: s3://ochre-weights///)") ochreWeightsPullCmd.Flags().String("hf-token", "", "Hugging Face token for a gated repo (falls back to HF_TOKEN, then a stored platform credential)") - _ = ochreWeightsPullCmd.MarkFlagRequired("model-id") + mustMarkFlagRequired(ochreWeightsPullCmd, "model-id") ochreCredentialsSetCmd.Flags().String("vendor", "", "Vendor to store a credential for, e.g. huggingface (required)") ochreCredentialsSetCmd.Flags().String("account", "", "Account ID to store the credential under (default: platform account)") ochreCredentialsSetCmd.Flags().String("token", "", "Must be '-': reads the secret from stdin (required)") - _ = ochreCredentialsSetCmd.MarkFlagRequired("vendor") - _ = ochreCredentialsSetCmd.MarkFlagRequired("token") + mustMarkFlagRequired(ochreCredentialsSetCmd, "vendor") + mustMarkFlagRequired(ochreCredentialsSetCmd, "token") } // requiredWeightsFiles are the fixed-name Hugging Face artefacts stage @@ -1172,11 +1172,11 @@ func init() { c.Flags().String("account-id", "", "12-digit account ID to change (required)") c.Flags().String("model-id", "", "Model ID to change (e.g. meta.llama3-2-1b-instruct-v1:0)") c.Flags().Bool("all-models", false, "Apply to every model in the platform catalog") - _ = c.MarkFlagRequired("account-id") + mustMarkFlagRequired(c, "account-id") } adminOchreAccessListCmd.Flags().String("account-id", "", "12-digit account ID to inspect (required)") - _ = adminOchreAccessListCmd.MarkFlagRequired("account-id") + mustMarkFlagRequired(adminOchreAccessListCmd, "account-id") } // ochreAccessStore connects to the cluster and returns the grant store along diff --git a/cmd/spinifex/cmd/admin_ochre_appliance.go b/spinifex/operator/cli/admin_ochre_appliance.go similarity index 99% rename from cmd/spinifex/cmd/admin_ochre_appliance.go rename to spinifex/operator/cli/admin_ochre_appliance.go index 2db1d6dda..6ae003389 100644 --- a/cmd/spinifex/cmd/admin_ochre_appliance.go +++ b/spinifex/operator/cli/admin_ochre_appliance.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_ochre_appliance_test.go b/spinifex/operator/cli/admin_ochre_appliance_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_ochre_appliance_test.go rename to spinifex/operator/cli/admin_ochre_appliance_test.go index 396bd10b7..51e6201c6 100644 --- a/cmd/spinifex/cmd/admin_ochre_appliance_test.go +++ b/spinifex/operator/cli/admin_ochre_appliance_test.go @@ -2,7 +2,7 @@ // exported surface to drive them through. // //test:in-package -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_ochre_endpoint.go b/spinifex/operator/cli/admin_ochre_endpoint.go similarity index 98% rename from cmd/spinifex/cmd/admin_ochre_endpoint.go rename to spinifex/operator/cli/admin_ochre_endpoint.go index 6ea569956..e114e6eb8 100644 --- a/cmd/spinifex/cmd/admin_ochre_endpoint.go +++ b/spinifex/operator/cli/admin_ochre_endpoint.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" @@ -72,15 +72,15 @@ func init() { ochreEndpointEnsureCmd.Flags().String("model-id", "", "Catalog model ID to bring an endpoint up for (required)") ochreEndpointEnsureCmd.Flags().Bool("wait", false, "Poll until the endpoint is READY and report the elapsed cold start") ochreEndpointEnsureCmd.Flags().Duration("timeout", defaultEndpointWaitTimeout, "How long --wait polls before giving up") - _ = ochreEndpointEnsureCmd.MarkFlagRequired("model-id") + mustMarkFlagRequired(ochreEndpointEnsureCmd, "model-id") ochreEndpointDescribeCmd.Flags().String("model-id", "", "Model ID to describe (required)") ochreEndpointDescribeCmd.Flags().String("account", "", "Account ID scoping the lookup (defaults to the shared platform account; set to see a pinned provisioned-throughput endpoint)") - _ = ochreEndpointDescribeCmd.MarkFlagRequired("model-id") + mustMarkFlagRequired(ochreEndpointDescribeCmd, "model-id") ochreEndpointDeleteCmd.Flags().String("model-id", "", "Model ID whose endpoint to tear down (required)") ochreEndpointDeleteCmd.Flags().String("account", "", "Account ID scoping the target (defaults to the shared platform account; set to tear down a pinned provisioned-throughput endpoint)") - _ = ochreEndpointDeleteCmd.MarkFlagRequired("model-id") + mustMarkFlagRequired(ochreEndpointDeleteCmd, "model-id") } const ( diff --git a/cmd/spinifex/cmd/admin_ochre_endpoint_test.go b/spinifex/operator/cli/admin_ochre_endpoint_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_ochre_endpoint_test.go rename to spinifex/operator/cli/admin_ochre_endpoint_test.go index 0dd63864b..6813cbde4 100644 --- a/cmd/spinifex/cmd/admin_ochre_endpoint_test.go +++ b/spinifex/operator/cli/admin_ochre_endpoint_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_ochre_pull_test.go b/spinifex/operator/cli/admin_ochre_pull_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_ochre_pull_test.go rename to spinifex/operator/cli/admin_ochre_pull_test.go index f3c844b2a..09ce947ac 100644 --- a/cmd/spinifex/cmd/admin_ochre_pull_test.go +++ b/spinifex/operator/cli/admin_ochre_pull_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_ochre_test.go b/spinifex/operator/cli/admin_ochre_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_ochre_test.go rename to spinifex/operator/cli/admin_ochre_test.go index 926e961a5..1891be1c2 100644 --- a/cmd/spinifex/cmd/admin_ochre_test.go +++ b/spinifex/operator/cli/admin_ochre_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/admin_ochre_vector.go b/spinifex/operator/cli/admin_ochre_vector.go similarity index 97% rename from cmd/spinifex/cmd/admin_ochre_vector.go rename to spinifex/operator/cli/admin_ochre_vector.go index c2898a3a3..5e5a3c6c3 100644 --- a/cmd/spinifex/cmd/admin_ochre_vector.go +++ b/spinifex/operator/cli/admin_ochre_vector.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" @@ -133,8 +133,8 @@ func init() { ochreVectorIndexCreateCmd.Flags().String("name", "", "Human-readable index name (required)") ochreVectorIndexCreateCmd.Flags().Int("dimension", 0, "Embedding vector dimension (required)") ochreVectorIndexCreateCmd.Flags().String("model", defaultVectorEmbeddingModel, "Embedding model this index is pinned to") - _ = ochreVectorIndexCreateCmd.MarkFlagRequired("name") - _ = ochreVectorIndexCreateCmd.MarkFlagRequired("dimension") + mustMarkFlagRequired(ochreVectorIndexCreateCmd, "name") + mustMarkFlagRequired(ochreVectorIndexCreateCmd, "dimension") ochreVectorIngestCmd.Flags().String("index", "", "Target index ID (required)") ochreVectorIngestCmd.Flags().String("bucket", "", "Source predastore bucket (required)") @@ -142,24 +142,24 @@ func init() { ochreVectorIngestCmd.Flags().Int("chunk-size", 0, "Chunk size in runes (0 takes the package default)") ochreVectorIngestCmd.Flags().Int("chunk-overlap", 0, "Chunk overlap in runes (0 takes the package default)") ochreVectorIngestCmd.Flags().StringToString("meta", nil, "Static metadata tag stamped on every ingested row, k=v (repeatable)") - _ = ochreVectorIngestCmd.MarkFlagRequired("index") - _ = ochreVectorIngestCmd.MarkFlagRequired("bucket") + mustMarkFlagRequired(ochreVectorIngestCmd, "index") + mustMarkFlagRequired(ochreVectorIngestCmd, "bucket") ochreVectorQueryCmd.Flags().String("index", "", "Index ID to query (required)") ochreVectorQueryCmd.Flags().String("text", "", "Query text, embedded server-side against the index's pinned model (required)") ochreVectorQueryCmd.Flags().Int("k", 0, "Number of results (0 takes the service default; hard-capped at 100)") ochreVectorQueryCmd.Flags().String("filter", "", "JSON-encoded metadata Filter AST, e.g. {\"Op\":\"equals\",\"Key\":\"category\",\"Value\":\"faq\"}") ochreVectorQueryCmd.Flags().Bool("json", false, "Print full, untruncated results as JSON") - _ = ochreVectorQueryCmd.MarkFlagRequired("index") - _ = ochreVectorQueryCmd.MarkFlagRequired("text") + mustMarkFlagRequired(ochreVectorQueryCmd, "index") + mustMarkFlagRequired(ochreVectorQueryCmd, "text") ochreVectorBackupCmd.Flags().String("account", "", "Account ID to back up (required)") - _ = ochreVectorBackupCmd.MarkFlagRequired("account") + mustMarkFlagRequired(ochreVectorBackupCmd, "account") ochreVectorRestoreCmd.Flags().String("account", "", "Account ID to restore into (required)") ochreVectorRestoreCmd.Flags().String("from", "", "Backup object key to restore from, as printed by 'backup' (required)") - _ = ochreVectorRestoreCmd.MarkFlagRequired("account") - _ = ochreVectorRestoreCmd.MarkFlagRequired("from") + mustMarkFlagRequired(ochreVectorRestoreCmd, "account") + mustMarkFlagRequired(ochreVectorRestoreCmd, "from") } // vectorServiceFn indirects the NATS-backed client so the Run functions' diff --git a/cmd/spinifex/cmd/admin_ochre_vector_test.go b/spinifex/operator/cli/admin_ochre_vector_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_ochre_vector_test.go rename to spinifex/operator/cli/admin_ochre_vector_test.go index bbeace39f..2b929f187 100644 --- a/cmd/spinifex/cmd/admin_ochre_vector_test.go +++ b/spinifex/operator/cli/admin_ochre_vector_test.go @@ -2,7 +2,7 @@ // exported surface to drive them through. // //test:in-package -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/admin_preflight.go b/spinifex/operator/cli/admin_preflight.go similarity index 99% rename from cmd/spinifex/cmd/admin_preflight.go rename to spinifex/operator/cli/admin_preflight.go index 10490525d..a72d3e1b5 100644 --- a/cmd/spinifex/cmd/admin_preflight.go +++ b/spinifex/operator/cli/admin_preflight.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "fmt" diff --git a/cmd/spinifex/cmd/admin_principal.go b/spinifex/operator/cli/admin_principal.go similarity index 99% rename from cmd/spinifex/cmd/admin_principal.go rename to spinifex/operator/cli/admin_principal.go index 0da530ee6..e3d383e04 100644 --- a/cmd/spinifex/cmd/admin_principal.go +++ b/spinifex/operator/cli/admin_principal.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "encoding/json" diff --git a/cmd/spinifex/cmd/admin_principal_iam_test.go b/spinifex/operator/cli/admin_principal_iam_test.go similarity index 90% rename from cmd/spinifex/cmd/admin_principal_iam_test.go rename to spinifex/operator/cli/admin_principal_iam_test.go index 08fdc0334..799c5af42 100644 --- a/cmd/spinifex/cmd/admin_principal_iam_test.go +++ b/spinifex/operator/cli/admin_principal_iam_test.go @@ -1,4 +1,4 @@ -package cmd_test +package cli_test import ( "errors" @@ -6,8 +6,8 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/mulgadc/spinifex/spinifex/operator/cli" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -79,14 +79,14 @@ func (s *principalIAMStub) DeleteAccessKey(_ string, input *iam.DeleteAccessKeyI // from an earlier name would keep authorising what the operator just removed. func TestCreateDropsAnInlinePolicyItDidNotWrite(t *testing.T) { svc := &principalIAMStub{inlinePolicies: []string{ - "operator-admin-methods", cmd.AdminPrincipalPolicyName, "signup-createaccount", + "operator-admin-methods", cli.AdminPrincipalPolicyName, "signup-createaccount", }} - dropped, err := cmd.DropStalePrincipalPolicies(svc, superAdminAccountID, "operator") + dropped, err := cli.DropStalePrincipalPolicies(svc, superAdminAccountID, "operator") require.NoError(t, err) assert.Equal(t, []string{"operator-admin-methods", "signup-createaccount"}, dropped) - assert.NotContains(t, svc.deletedPolicies, cmd.AdminPrincipalPolicyName, + assert.NotContains(t, svc.deletedPolicies, cli.AdminPrincipalPolicyName, "the policy the command owns must survive its own rewrite") } @@ -95,7 +95,7 @@ func TestCreateDropsAnInlinePolicyItDidNotWrite(t *testing.T) { func TestRevokeRemovesEveryAccessKey(t *testing.T) { svc := &principalIAMStub{accessKeys: []string{"AKIAONE", "AKIATWO"}} - revoked, err := cmd.RevokePrincipalKeys(svc, superAdminAccountID, "operator") + revoked, err := cli.RevokePrincipalKeys(svc, superAdminAccountID, "operator") require.NoError(t, err) assert.Equal(t, []string{"AKIAONE", "AKIATWO"}, revoked) @@ -107,7 +107,7 @@ func TestRevokeRemovesEveryAccessKey(t *testing.T) { func TestRevokeFailsRatherThanGuessing(t *testing.T) { svc := &principalIAMStub{listKeysErr: errors.New("kv unavailable")} - _, err := cmd.RevokePrincipalKeys(svc, superAdminAccountID, "operator") + _, err := cli.RevokePrincipalKeys(svc, superAdminAccountID, "operator") require.Error(t, err) assert.Empty(t, svc.deletedKeys) @@ -122,7 +122,7 @@ func TestDescribePrincipalReportsScopedGrants(t *testing.T) { accessKeys: []string{"AKIAONE"}, } - row, err := cmd.DescribePrincipal(svc, superAdminAccountID, "signup") + row, err := cli.DescribePrincipal(svc, superAdminAccountID, "signup") require.NoError(t, err) assert.Equal(t, "signup", row.UserName) @@ -135,7 +135,7 @@ func TestDescribePrincipalReportsScopedGrants(t *testing.T) { func TestDescribePrincipalMarksAnAttachedPolicy(t *testing.T) { svc := &principalIAMStub{attachedPolicies: []string{handlers_iam.AdminPolicyName}} - row, err := cmd.DescribePrincipal(svc, superAdminAccountID, handlers_iam.AdminUserName) + row, err := cli.DescribePrincipal(svc, superAdminAccountID, handlers_iam.AdminUserName) require.NoError(t, err) assert.Equal(t, []string{handlers_iam.AdminPolicyName + " (attached)"}, row.Grants) diff --git a/cmd/spinifex/cmd/admin_principal_test.go b/spinifex/operator/cli/admin_principal_test.go similarity index 82% rename from cmd/spinifex/cmd/admin_principal_test.go rename to spinifex/operator/cli/admin_principal_test.go index 46ed88321..f92b272a2 100644 --- a/cmd/spinifex/cmd/admin_principal_test.go +++ b/spinifex/operator/cli/admin_principal_test.go @@ -1,13 +1,13 @@ -package cmd_test +package cli_test import ( "encoding/json" "strings" "testing" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" "github.com/mulgadc/spinifex/spinifex/gateway" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/mulgadc/spinifex/spinifex/operator/cli" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -38,7 +38,7 @@ func policyActions(t *testing.T, document string) []string { // cover the surface the gateway actually serves rather than a list that can // fall behind it. func TestPrincipalGrantsDefaultToEveryAdminMethod(t *testing.T) { - grants, err := cmd.ResolvePrincipalGrants(nil) + grants, err := cli.ResolvePrincipalGrants(nil) require.NoError(t, err) var want []string @@ -51,7 +51,7 @@ func TestPrincipalGrantsDefaultToEveryAdminMethod(t *testing.T) { // Scoping is the whole point of the flag: a signup credential holds exactly // CreateAccount, so a leaked key cannot remove a tenant. func TestPrincipalGrantsScopeToTheRequestedMethods(t *testing.T) { - grants, err := cmd.ResolvePrincipalGrants([]string{"CreateAccount"}) + grants, err := cli.ResolvePrincipalGrants([]string{"CreateAccount"}) require.NoError(t, err) assert.Equal(t, []string{"spinifex:CreateAccount"}, grants) @@ -60,7 +60,7 @@ func TestPrincipalGrantsScopeToTheRequestedMethods(t *testing.T) { // Operators write the method either way, and a duplicate is a typo rather than // a reason to refuse. func TestPrincipalGrantsAcceptQualifiedAndRepeatedNames(t *testing.T) { - grants, err := cmd.ResolvePrincipalGrants([]string{ + grants, err := cli.ResolvePrincipalGrants([]string{ "spinifex:ListAccounts", "listaccounts", " DeleteAccount ", }) require.NoError(t, err) @@ -71,7 +71,7 @@ func TestPrincipalGrantsAcceptQualifiedAndRepeatedNames(t *testing.T) { // A misspelt method must fail rather than mint a key that silently authorises // nothing — the caller would only find out at the first AccessDenied. func TestPrincipalGrantsRejectAnUnknownMethod(t *testing.T) { - _, err := cmd.ResolvePrincipalGrants([]string{"DeleteEverything"}) + _, err := cli.ResolvePrincipalGrants([]string{"DeleteEverything"}) require.Error(t, err) assert.Contains(t, err.Error(), "DeleteEverything") @@ -81,10 +81,10 @@ func TestPrincipalGrantsRejectAnUnknownMethod(t *testing.T) { // A wildcard here would authorise whatever is added to the surface next with a // key minted before it existed. func TestPrincipalPolicyNamesEveryMethod(t *testing.T) { - grants, err := cmd.ResolvePrincipalGrants(nil) + grants, err := cli.ResolvePrincipalGrants(nil) require.NoError(t, err) - actions := policyActions(t, cmd.PrincipalPolicyDocument(grants)) + actions := policyActions(t, cli.PrincipalPolicyDocument(grants)) assert.ElementsMatch(t, grants, actions) for _, action := range actions { @@ -95,11 +95,11 @@ func TestPrincipalPolicyNamesEveryMethod(t *testing.T) { // The listing reads back what the create path wrote, so a principal's grants // can be audited without decoding the document by hand. func TestPrincipalPolicyActionsRoundTrip(t *testing.T) { - grants, err := cmd.ResolvePrincipalGrants([]string{"CreateAccount", "ListAccounts"}) + grants, err := cli.ResolvePrincipalGrants([]string{"CreateAccount", "ListAccounts"}) require.NoError(t, err) assert.Equal(t, []string{"CreateAccount", "ListAccounts"}, - cmd.PrincipalPolicyActions(cmd.PrincipalPolicyDocument(grants))) + cli.PrincipalPolicyActions(cli.PrincipalPolicyDocument(grants))) } // A NotAction grant covers everything but its list, so rendering only Action @@ -111,37 +111,37 @@ func TestPrincipalPolicyActionsRenderNotAction(t *testing.T) { {"Effect":"Deny","NotAction":"spinifex:ListAccounts","Resource":"*"}]}` assert.Equal(t, []string{"ListAccounts", "NotAction(DeleteAccount, CreateAccount)"}, - cmd.PrincipalPolicyActions(document)) + cli.PrincipalPolicyActions(document)) } // A document nobody can parse is reported as no grants rather than as a guess. func TestPrincipalPolicyActionsIgnoreAnUndecodableDocument(t *testing.T) { - assert.Empty(t, cmd.PrincipalPolicyActions("{not json")) + assert.Empty(t, cli.PrincipalPolicyActions("{not json")) } // Rotating the super-admin account's own administrator would revoke the // bootstrap credential the operator is holding. func TestPrincipalNameRefusesTheAccountAdministrator(t *testing.T) { - err := cmd.ValidatePrincipalName(handlers_iam.AdminUserName) + err := cli.ValidatePrincipalName(handlers_iam.AdminUserName) require.Error(t, err) assert.Contains(t, err.Error(), handlers_iam.AdminUserName) } func TestPrincipalNameRejectsUnusableNames(t *testing.T) { - assert.Error(t, cmd.ValidatePrincipalName("")) - assert.Error(t, cmd.ValidatePrincipalName("has space")) - assert.Error(t, cmd.ValidatePrincipalName(strings.Repeat("a", 65))) + assert.Error(t, cli.ValidatePrincipalName("")) + assert.Error(t, cli.ValidatePrincipalName("has space")) + assert.Error(t, cli.ValidatePrincipalName(strings.Repeat("a", 65))) - assert.NoError(t, cmd.ValidatePrincipalName("signup")) - assert.NoError(t, cmd.ValidatePrincipalName("ops-team_1@example.com")) + assert.NoError(t, cli.ValidatePrincipalName("signup")) + assert.NoError(t, cli.ValidatePrincipalName("ops-team_1@example.com")) } // The listing prints one line per principal, including a revoked credential // that holds no key — an operator has to see it to know it is still there. func TestPrintPrincipalTableRendersEveryPrincipal(t *testing.T) { output := captureOutput(t, func() { - cmd.PrintPrincipalTable("000000000001", []cmd.PrincipalRowSpec{ + cli.PrintPrincipalTable("000000000001", []cli.PrincipalRowSpec{ {UserName: "admin", Grants: []string{"AdministratorAccess (attached)"}, Keys: 1}, {UserName: "signup", Grants: []string{"CreateAccount"}, Keys: 1}, {UserName: "retired", Keys: 0}, @@ -159,7 +159,7 @@ func TestPrintPrincipalTableRendersEveryPrincipal(t *testing.T) { // a rendering fault rather than as an answer. func TestPrintPrincipalTableMarksAnEmptyGrant(t *testing.T) { output := captureOutput(t, func() { - cmd.PrintPrincipalTable("000000000001", []cmd.PrincipalRowSpec{{UserName: "retired"}}) + cli.PrintPrincipalTable("000000000001", []cli.PrincipalRowSpec{{UserName: "retired"}}) }) assert.Contains(t, output, "-") @@ -169,7 +169,7 @@ func TestPrintPrincipalTableMarksAnEmptyGrant(t *testing.T) { // TERMINATING reads the same either way — which is how a stuck teardown is // noticed. func TestAccountSummariesCarryStatus(t *testing.T) { - summaries := cmd.AccountSummaries([]*handlers_iam.Account{ + summaries := cli.AccountSummaries([]*handlers_iam.Account{ {AccountID: "000000000042", AccountName: "t@example.com", Status: handlers_iam.AccountStatusActive}, nil, {AccountID: "000000000043", AccountName: "u@example.com", Status: handlers_iam.AccountStatusTerminating}, @@ -181,7 +181,7 @@ func TestAccountSummariesCarryStatus(t *testing.T) { func TestPrintAccountTableRendersEveryAccount(t *testing.T) { output := captureOutput(t, func() { - cmd.PrintAccountTable([]gateway.AccountSummary{ + cli.PrintAccountTable([]gateway.AccountSummary{ { AccountID: "000000000042", AccountName: "tenant@example.com", @@ -199,7 +199,7 @@ func TestPrintAccountTableRendersEveryAccount(t *testing.T) { // An empty listing must say so. A bare header reads as truncated output. func TestPrintAccountTableSaysWhenThereAreNone(t *testing.T) { - output := captureOutput(t, func() { cmd.PrintAccountTable(nil) }) + output := captureOutput(t, func() { cli.PrintAccountTable(nil) }) assert.Contains(t, output, "No accounts found") } diff --git a/cmd/spinifex/cmd/admin_remote.go b/spinifex/operator/cli/admin_remote.go similarity index 98% rename from cmd/spinifex/cmd/admin_remote.go rename to spinifex/operator/cli/admin_remote.go index d0645887a..16a819eed 100644 --- a/cmd/spinifex/cmd/admin_remote.go +++ b/spinifex/operator/cli/admin_remote.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bytes" @@ -246,7 +246,11 @@ func newClientToken() string { // command works against a cluster's self-signed CA and against a public // certificate. An unreadable bundle is an error, never a silent downgrade. func adminHTTPClient(caBundle string) (*http.Client, error) { - transport := http.DefaultTransport.(*http.Transport).Clone() + base, ok := http.DefaultTransport.(*http.Transport) + if !ok { + return nil, fmt.Errorf("http.DefaultTransport is %T, not *http.Transport", http.DefaultTransport) + } + transport := base.Clone() if caBundle != "" { pem, err := os.ReadFile(caBundle) if err != nil { diff --git a/cmd/spinifex/cmd/admin_remote_test.go b/spinifex/operator/cli/admin_remote_test.go similarity index 90% rename from cmd/spinifex/cmd/admin_remote_test.go rename to spinifex/operator/cli/admin_remote_test.go index 8744278a2..ae3f5fa73 100644 --- a/cmd/spinifex/cmd/admin_remote_test.go +++ b/spinifex/operator/cli/admin_remote_test.go @@ -1,4 +1,4 @@ -package cmd_test +package cli_test import ( "encoding/pem" @@ -10,9 +10,9 @@ import ( "strings" "testing" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" "github.com/mulgadc/spinifex/spinifex/bootstrap/config" "github.com/mulgadc/spinifex/spinifex/gateway" + "github.com/mulgadc/spinifex/spinifex/operator/cli" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -77,7 +77,7 @@ func TestTrustsWholeAccount(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.want, cmd.TrustsWholeAccount(tc.document, auditAccountID)) + assert.Equal(t, tc.want, cli.TrustsWholeAccount(tc.document, auditAccountID)) }) } } @@ -85,8 +85,8 @@ func TestTrustsWholeAccount(t *testing.T) { func TestNewClientTokenIsAccepted(t *testing.T) { valid := regexp.MustCompile(`^[A-Za-z0-9_-]{32,128}$`) - first := cmd.NewClientToken() - second := cmd.NewClientToken() + first := cli.NewClientToken() + second := cli.NewClientToken() assert.Regexp(t, valid, first) assert.NotEqual(t, first, second) @@ -106,7 +106,7 @@ func TestLocalGatewayEndpoint(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { node := config.Config{AWSGW: config.AWSGWConfig{Host: tc.host}} - assert.Equal(t, tc.want, cmd.LocalGatewayEndpoint(node)) + assert.Equal(t, tc.want, cli.LocalGatewayEndpoint(node)) }) } } @@ -114,12 +114,12 @@ func TestLocalGatewayEndpoint(t *testing.T) { // An unreadable or empty CA bundle must be an error, never a silent fallback // to an unverified connection. func TestAdminHTTPClientRejectsBadCABundle(t *testing.T) { - _, err := cmd.AdminHTTPClient(filepath.Join(t.TempDir(), "absent.pem")) + _, err := cli.AdminHTTPClient(filepath.Join(t.TempDir(), "absent.pem")) assert.Error(t, err) empty := filepath.Join(t.TempDir(), "empty.pem") require.NoError(t, os.WriteFile(empty, []byte("not a certificate"), 0o600)) - _, err = cmd.AdminHTTPClient(empty) + _, err = cli.AdminHTTPClient(empty) assert.Error(t, err) } @@ -159,8 +159,8 @@ func TestCreateAccountRemoteSignsAndPosts(t *testing.T) { var got http.Request srv := newAdminTestServer(t, http.StatusOK, createAccountOK, &got) - out, err := cmd.CreateAccountRemote(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), + out, err := cli.CreateAccountRemote(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), gateway.CreateAccountRequest{Name: "ben@example.com", ClientToken: strings.Repeat("a", 32)}) require.NoError(t, err) @@ -184,8 +184,8 @@ func TestCreateAccountRemoteTrimsEndpointSlash(t *testing.T) { var got http.Request srv := newAdminTestServer(t, http.StatusOK, createAccountOK, &got) - _, err := cmd.CreateAccountRemote(t.Context(), - cmd.AdminTarget(srv.URL+"/", "us-west-1", writeServerCA(t, srv)), + _, err := cli.CreateAccountRemote(t.Context(), + cli.AdminTarget(srv.URL+"/", "us-west-1", writeServerCA(t, srv)), gateway.CreateAccountRequest{Name: "ben@example.com", ClientToken: strings.Repeat("a", 32)}) require.NoError(t, err) assert.Equal(t, "/admin/CreateAccount", got.URL.Path) @@ -199,14 +199,14 @@ func TestCreateAccountRemoteSurfacesGatewayError(t *testing.T) { srv := newAdminTestServer(t, http.StatusConflict, `{"error":{"code":"AccountAlreadyExists","message":"taken"},"requestId":"req-1"}`, &got) - _, err := cmd.CreateAccountRemote(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), + _, err := cli.CreateAccountRemote(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", writeServerCA(t, srv)), gateway.CreateAccountRequest{Name: "ben@example.com", ClientToken: strings.Repeat("a", 32)}) require.Error(t, err) assert.Contains(t, err.Error(), "AccountAlreadyExists") assert.Contains(t, err.Error(), "req-1") - assert.False(t, cmd.RetryableAdminError(err), "a taken name is not fixed by retrying") + assert.False(t, cli.RetryableAdminError(err), "a taken name is not fixed by retrying") } // An untrusted certificate must fail rather than fall back to no verification. @@ -217,8 +217,8 @@ func TestCreateAccountRemoteRefusesUntrustedServer(t *testing.T) { var got http.Request srv := newAdminTestServer(t, http.StatusOK, createAccountOK, &got) - _, err := cmd.CreateAccountRemote(t.Context(), - cmd.AdminTarget(srv.URL, "us-west-1", ""), + _, err := cli.CreateAccountRemote(t.Context(), + cli.AdminTarget(srv.URL, "us-west-1", ""), gateway.CreateAccountRequest{Name: "ben@example.com", ClientToken: strings.Repeat("a", 32)}) assert.Error(t, err) } @@ -276,10 +276,10 @@ func TestDecodeAdminErrorRetryability(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - err := cmd.DecodeAdminError(tc.status, []byte(tc.body)) + err := cli.DecodeAdminError(tc.status, []byte(tc.body)) require.Error(t, err) assert.Contains(t, err.Error(), tc.wantMessage) - assert.Equal(t, tc.wantRetry, cmd.RetryableAdminError(err)) + assert.Equal(t, tc.wantRetry, cli.RetryableAdminError(err)) }) } } diff --git a/cmd/spinifex/cmd/admin_test.go b/spinifex/operator/cli/admin_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_test.go rename to spinifex/operator/cli/admin_test.go index 032365212..16e57beef 100644 --- a/cmd/spinifex/cmd/admin_test.go +++ b/spinifex/operator/cli/admin_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/admin_upgrade.go b/spinifex/operator/cli/admin_upgrade.go similarity index 99% rename from cmd/spinifex/cmd/admin_upgrade.go rename to spinifex/operator/cli/admin_upgrade.go index 559e322c7..ba1d67bab 100644 --- a/cmd/spinifex/cmd/admin_upgrade.go +++ b/spinifex/operator/cli/admin_upgrade.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bufio" diff --git a/cmd/spinifex/cmd/admin_upgrade_test.go b/spinifex/operator/cli/admin_upgrade_test.go similarity index 99% rename from cmd/spinifex/cmd/admin_upgrade_test.go rename to spinifex/operator/cli/admin_upgrade_test.go index 1e6689121..d462aca73 100644 --- a/cmd/spinifex/cmd/admin_upgrade_test.go +++ b/spinifex/operator/cli/admin_upgrade_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/cluster.go b/spinifex/operator/cli/cluster.go similarity index 99% rename from cmd/spinifex/cmd/cluster.go rename to spinifex/operator/cli/cluster.go index 2250a33cf..6988f679b 100644 --- a/cmd/spinifex/cmd/cluster.go +++ b/spinifex/operator/cli/cluster.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "encoding/json" diff --git a/cmd/spinifex/cmd/cluster_test.go b/spinifex/operator/cli/cluster_test.go similarity index 99% rename from cmd/spinifex/cmd/cluster_test.go rename to spinifex/operator/cli/cluster_test.go index dc479e063..81869966e 100644 --- a/cmd/spinifex/cmd/cluster_test.go +++ b/spinifex/operator/cli/cluster_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/export_test.go b/spinifex/operator/cli/export_test.go similarity index 99% rename from cmd/spinifex/cmd/export_test.go rename to spinifex/operator/cli/export_test.go index 9a22769ba..dc5ab8f0b 100644 --- a/cmd/spinifex/cmd/export_test.go +++ b/spinifex/operator/cli/export_test.go @@ -1,7 +1,7 @@ //test:in-package — the file exists to expose unexported identifiers to the // external cmd_test package, which it can only do from inside cmd. -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/get.go b/spinifex/operator/cli/get.go similarity index 91% rename from cmd/spinifex/cmd/get.go rename to spinifex/operator/cli/get.go index ae618dee8..f0a37f6bb 100644 --- a/cmd/spinifex/cmd/get.go +++ b/spinifex/operator/cli/get.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "encoding/json" @@ -229,7 +229,15 @@ func runGetNodes(cmd *cobra.Command, args []string) { } } - pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(tableData).Render() + // Render discards its write error, so write the table here to fail on it. + rendered, err := pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(tableData).Srender() + if err == nil { + _, err = fmt.Fprintln(os.Stdout, rendered) + } + if err != nil { + fmt.Fprintf(os.Stderr, "Error: render table: %v\n", err) + os.Exit(1) + } } func runGetVMs(cmd *cobra.Command, args []string) { @@ -305,5 +313,13 @@ func runGetVMs(cmd *cobra.Command, args []string) { }) } - pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(tableData).Render() + // Render discards its write error, so write the table here to fail on it. + rendered, err := pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(tableData).Srender() + if err == nil { + _, err = fmt.Fprintln(os.Stdout, rendered) + } + if err != nil { + fmt.Fprintf(os.Stderr, "Error: render table: %v\n", err) + os.Exit(1) + } } diff --git a/cmd/spinifex/cmd/get_dhcp_leases.go b/spinifex/operator/cli/get_dhcp_leases.go similarity index 99% rename from cmd/spinifex/cmd/get_dhcp_leases.go rename to spinifex/operator/cli/get_dhcp_leases.go index 654802b3e..0c1cbf414 100644 --- a/cmd/spinifex/cmd/get_dhcp_leases.go +++ b/spinifex/operator/cli/get_dhcp_leases.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/get_dhcp_leases_test.go b/spinifex/operator/cli/get_dhcp_leases_test.go similarity index 99% rename from cmd/spinifex/cmd/get_dhcp_leases_test.go rename to spinifex/operator/cli/get_dhcp_leases_test.go index 31985b23e..2d16461f3 100644 --- a/cmd/spinifex/cmd/get_dhcp_leases_test.go +++ b/spinifex/operator/cli/get_dhcp_leases_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "encoding/json" diff --git a/cmd/spinifex/cmd/get_test.go b/spinifex/operator/cli/get_test.go similarity index 99% rename from cmd/spinifex/cmd/get_test.go rename to spinifex/operator/cli/get_test.go index fe2ff6e3b..009a246a6 100644 --- a/cmd/spinifex/cmd/get_test.go +++ b/spinifex/operator/cli/get_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "testing" diff --git a/cmd/spinifex/cmd/jetstream_store.go b/spinifex/operator/cli/jetstream_store.go similarity index 99% rename from cmd/spinifex/cmd/jetstream_store.go rename to spinifex/operator/cli/jetstream_store.go index 8592c2765..b968a667a 100644 --- a/cmd/spinifex/cmd/jetstream_store.go +++ b/spinifex/operator/cli/jetstream_store.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "bytes" diff --git a/cmd/spinifex/cmd/jetstream_store_test.go b/spinifex/operator/cli/jetstream_store_test.go similarity index 99% rename from cmd/spinifex/cmd/jetstream_store_test.go rename to spinifex/operator/cli/jetstream_store_test.go index 6c890c334..e4e8ec4ec 100644 --- a/cmd/spinifex/cmd/jetstream_store_test.go +++ b/spinifex/operator/cli/jetstream_store_test.go @@ -1,7 +1,7 @@ //test:in-package: swaps the unexported procRoot for a fake /proc tree and asserts on // the store helpers the init and join commands call. -package cmd +package cli import ( "os" diff --git a/cmd/spinifex/cmd/nats_server_name_test.go b/spinifex/operator/cli/nats_server_name_test.go similarity index 99% rename from cmd/spinifex/cmd/nats_server_name_test.go rename to spinifex/operator/cli/nats_server_name_test.go index c8d9512db..541d7f26a 100644 --- a/cmd/spinifex/cmd/nats_server_name_test.go +++ b/spinifex/operator/cli/nats_server_name_test.go @@ -1,7 +1,7 @@ //test:in-package — the embedded nats.conf template it checks is unexported, // and exporting it would widen the package's API for a test's convenience. -package cmd +package cli import ( "strings" diff --git a/cmd/spinifex/cmd/northstar_template_test.go b/spinifex/operator/cli/northstar_template_test.go similarity index 99% rename from cmd/spinifex/cmd/northstar_template_test.go rename to spinifex/operator/cli/northstar_template_test.go index 750dc5c11..78fade1ea 100644 --- a/cmd/spinifex/cmd/northstar_template_test.go +++ b/spinifex/operator/cli/northstar_template_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "os" diff --git a/cmd/spinifex/cmd/paths.go b/spinifex/operator/cli/paths.go similarity index 99% rename from cmd/spinifex/cmd/paths.go rename to spinifex/operator/cli/paths.go index fe147ce26..c529c9223 100644 --- a/cmd/spinifex/cmd/paths.go +++ b/spinifex/operator/cli/paths.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "fmt" diff --git a/cmd/spinifex/cmd/paths_test.go b/spinifex/operator/cli/paths_test.go similarity index 99% rename from cmd/spinifex/cmd/paths_test.go rename to spinifex/operator/cli/paths_test.go index b1bd0cff8..4f68e1c9a 100644 --- a/cmd/spinifex/cmd/paths_test.go +++ b/spinifex/operator/cli/paths_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "os" diff --git a/cmd/spinifex/cmd/predastore_bind_test.go b/spinifex/operator/cli/predastore_bind_test.go similarity index 99% rename from cmd/spinifex/cmd/predastore_bind_test.go rename to spinifex/operator/cli/predastore_bind_test.go index b98bec22b..2a38c432c 100644 --- a/cmd/spinifex/cmd/predastore_bind_test.go +++ b/spinifex/operator/cli/predastore_bind_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "os" diff --git a/cmd/spinifex/cmd/predastore_planes_test.go b/spinifex/operator/cli/predastore_planes_test.go similarity index 99% rename from cmd/spinifex/cmd/predastore_planes_test.go rename to spinifex/operator/cli/predastore_planes_test.go index 4f923b65d..2a93009ea 100644 --- a/cmd/spinifex/cmd/predastore_planes_test.go +++ b/spinifex/operator/cli/predastore_planes_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "os" diff --git a/cmd/spinifex/cmd/register_imported_ami_snapshot_test.go b/spinifex/operator/cli/register_imported_ami_snapshot_test.go similarity index 99% rename from cmd/spinifex/cmd/register_imported_ami_snapshot_test.go rename to spinifex/operator/cli/register_imported_ami_snapshot_test.go index 356686aad..21fb7ee8b 100644 --- a/cmd/spinifex/cmd/register_imported_ami_snapshot_test.go +++ b/spinifex/operator/cli/register_imported_ami_snapshot_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" diff --git a/cmd/spinifex/cmd/root.go b/spinifex/operator/cli/root.go similarity index 81% rename from cmd/spinifex/cmd/root.go rename to spinifex/operator/cli/root.go index 2c7e093f2..b3eca04e8 100644 --- a/cmd/spinifex/cmd/root.go +++ b/spinifex/operator/cli/root.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "fmt" @@ -7,7 +7,6 @@ import ( "github.com/mulgadc/spinifex/spinifex/bootstrap/config" "github.com/spf13/cobra" - "github.com/spf13/viper" ) var ( @@ -83,39 +82,39 @@ func init() { // Global flags rootCmd.PersistentFlags().String("config", "", "config file (required)") - viper.BindEnv("config", "SPINIFEX_CONFIG_PATH") - viper.BindPFlag("config", rootCmd.PersistentFlags().Lookup("config")) + mustBindEnv("config", "SPINIFEX_CONFIG_PATH") + mustBindPFlag("config", rootCmd.PersistentFlags(), "config") // Authentication (access_key, secret) rootCmd.PersistentFlags().String("access-key", "", "AWS access key (overrides config file and env)") - viper.BindEnv("access-key", "SPINIFEX_ACCESS_KEY") - viper.BindPFlag("access-key", rootCmd.PersistentFlags().Lookup("access-key")) + mustBindEnv("access-key", "SPINIFEX_ACCESS_KEY") + mustBindPFlag("access-key", rootCmd.PersistentFlags(), "access-key") rootCmd.PersistentFlags().String("secret-key", "", "AWS secret key (overrides config file and env)") - viper.BindEnv("secret-key", "SPINIFEX_SECRET_KEY") - viper.BindPFlag("secret-key", rootCmd.PersistentFlags().Lookup("secret-key")) + mustBindEnv("secret-key", "SPINIFEX_SECRET_KEY") + mustBindPFlag("secret-key", rootCmd.PersistentFlags(), "secret-key") rootCmd.PersistentFlags().String("host", "", "AWS Endpoint (overrides config file and env)") - viper.BindEnv("host", "SPINIFEX_HOST") - viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("host")) + mustBindEnv("host", "SPINIFEX_HOST") + mustBindPFlag("host", rootCmd.PersistentFlags(), "host") // Viperblock config rootCmd.PersistentFlags().String("base-dir", "", "Viperblock base directory (overrides config file and env)") - viper.BindEnv("base-dir", "SPINIFEX_BASE_DIR") - viper.BindPFlag("base-dir", rootCmd.PersistentFlags().Lookup("base-dir")) + mustBindEnv("base-dir", "SPINIFEX_BASE_DIR") + mustBindPFlag("base-dir", rootCmd.PersistentFlags(), "base-dir") // NATS specific flags rootCmd.PersistentFlags().String("nats-host", "", "NATS server host (overrides config file and env)") - viper.BindEnv("nats-host", "SPINIFEX_NATS_HOST") - viper.BindPFlag("nats-host", rootCmd.PersistentFlags().Lookup("nats-host")) + mustBindEnv("nats-host", "SPINIFEX_NATS_HOST") + mustBindPFlag("nats-host", rootCmd.PersistentFlags(), "nats-host") rootCmd.PersistentFlags().String("nats-token", "", "NATS authentication token (overrides config file and env)") - viper.BindEnv("nats-token", "SPINIFEX_NATS_TOKEN") - viper.BindPFlag("nats-token", rootCmd.PersistentFlags().Lookup("nats-token")) + mustBindEnv("nats-token", "SPINIFEX_NATS_TOKEN") + mustBindPFlag("nats-token", rootCmd.PersistentFlags(), "nats-token") rootCmd.PersistentFlags().String("nats-subject", "", "NATS subscription subject (overrides config file and env)") - viper.BindEnv("nats-subject", "SPINIFEX_NATS_SUBJECT") - viper.BindPFlag("nats-subject", rootCmd.PersistentFlags().Lookup("nats-subject")) + mustBindEnv("nats-subject", "SPINIFEX_NATS_SUBJECT") + mustBindPFlag("nats-subject", rootCmd.PersistentFlags(), "nats-subject") // Bind flags to viper //viper.BindPFlag("nats.host", rootCmd.PersistentFlags().Lookup("nats-host")) diff --git a/cmd/spinifex/cmd/root_logger_test.go b/spinifex/operator/cli/root_logger_test.go similarity index 88% rename from cmd/spinifex/cmd/root_logger_test.go rename to spinifex/operator/cli/root_logger_test.go index 5a1de68b1..46dc42723 100644 --- a/cmd/spinifex/cmd/root_logger_test.go +++ b/spinifex/operator/cli/root_logger_test.go @@ -1,4 +1,4 @@ -package cmd_test +package cli_test import ( "fmt" @@ -6,7 +6,7 @@ import ( "strings" "testing" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" + "github.com/mulgadc/spinifex/spinifex/operator/cli" ) // handlerAtStartup samples the process logger once this test package's variables @@ -45,12 +45,12 @@ func TestInitCLILoggerLevels(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Setenv("SPX_LOG_LEVEL", tc.env) - defer cmd.SetVerboseFlag(tc.verbose)() + defer cli.SetVerboseFlag(tc.verbose)() - cmd.CLILogLevel.Set(slog.LevelError) - cmd.InitCLILogger() + cli.CLILogLevel.Set(slog.LevelError) + cli.InitCLILogger() - if got := cmd.CLILogLevel.Level(); got != tc.want { + if got := cli.CLILogLevel.Level(); got != tc.want { t.Errorf("level = %v, want %v", got, tc.want) } }) diff --git a/cmd/spinifex/cmd/service.go b/spinifex/operator/cli/service.go similarity index 88% rename from cmd/spinifex/cmd/service.go rename to spinifex/operator/cli/service.go index 15c255fb8..bb3b4c9cd 100644 --- a/cmd/spinifex/cmd/service.go +++ b/spinifex/operator/cli/service.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "context" @@ -991,82 +991,82 @@ func registerPredastoreNamespacedFlags() { // key's AutomaticEnv-derived name matches its explicit BindEnv target. Viper // checks AutomaticEnv first, so a bare key resolves SPINIFEX_HOST instead. func bindPredastoreNamespacedEnv() { - viper.BindEnv("predastore-host", "SPINIFEX_PREDASTORE_HOST") - viper.BindPFlag("predastore-host", predastoreCmd.PersistentFlags().Lookup("host")) + mustBindEnv("predastore-host", "SPINIFEX_PREDASTORE_HOST") + mustBindPFlag("predastore-host", predastoreCmd.PersistentFlags(), "host") - viper.BindEnv("predastore-base-path", "SPINIFEX_PREDASTORE_BASE_PATH") - viper.BindPFlag("predastore-base-path", predastoreCmd.PersistentFlags().Lookup("base-path")) + mustBindEnv("predastore-base-path", "SPINIFEX_PREDASTORE_BASE_PATH") + mustBindPFlag("predastore-base-path", predastoreCmd.PersistentFlags(), "base-path") - viper.BindEnv("predastore-config-path", "SPINIFEX_PREDASTORE_CONFIG_PATH") - viper.BindPFlag("predastore-config-path", predastoreCmd.PersistentFlags().Lookup("config-path")) + mustBindEnv("predastore-config-path", "SPINIFEX_PREDASTORE_CONFIG_PATH") + mustBindPFlag("predastore-config-path", predastoreCmd.PersistentFlags(), "config-path") } // bindPredastoreCollisionEnv namespaces predastore's port, tls-cert, tls-key, // encryption-key-file and host-id keys, which nats and awsgw also bind bare. // Each derived env name now matches its own BindEnv target. func bindPredastoreCollisionEnv() { - viper.BindEnv("predastore-port", "SPINIFEX_PREDASTORE_PORT") - viper.BindPFlag("predastore-port", predastoreCmd.PersistentFlags().Lookup("port")) + mustBindEnv("predastore-port", "SPINIFEX_PREDASTORE_PORT") + mustBindPFlag("predastore-port", predastoreCmd.PersistentFlags(), "port") - viper.BindEnv("predastore-tls-cert", "SPINIFEX_PREDASTORE_TLS_CERT") - viper.BindPFlag("predastore-tls-cert", predastoreCmd.PersistentFlags().Lookup("tls-cert")) + mustBindEnv("predastore-tls-cert", "SPINIFEX_PREDASTORE_TLS_CERT") + mustBindPFlag("predastore-tls-cert", predastoreCmd.PersistentFlags(), "tls-cert") - viper.BindEnv("predastore-tls-key", "SPINIFEX_PREDASTORE_TLS_KEY") - viper.BindPFlag("predastore-tls-key", predastoreCmd.PersistentFlags().Lookup("tls-key")) + mustBindEnv("predastore-tls-key", "SPINIFEX_PREDASTORE_TLS_KEY") + mustBindPFlag("predastore-tls-key", predastoreCmd.PersistentFlags(), "tls-key") - viper.BindEnv("predastore-encryption-key-file", "SPINIFEX_PREDASTORE_ENCRYPTION_KEY_FILE") - viper.BindPFlag("predastore-encryption-key-file", predastoreCmd.PersistentFlags().Lookup("encryption-key-file")) + mustBindEnv("predastore-encryption-key-file", "SPINIFEX_PREDASTORE_ENCRYPTION_KEY_FILE") + mustBindPFlag("predastore-encryption-key-file", predastoreCmd.PersistentFlags(), "encryption-key-file") - viper.BindEnv("predastore-host-id", "SPINIFEX_PREDASTORE_HOST_ID") - viper.BindPFlag("predastore-host-id", predastoreCmd.PersistentFlags().Lookup("host-id")) + mustBindEnv("predastore-host-id", "SPINIFEX_PREDASTORE_HOST_ID") + mustBindPFlag("predastore-host-id", predastoreCmd.PersistentFlags(), "host-id") } // bindNatsCollisionEnv namespaces nats's port, host and debug keys, which // awsgw also binds bare. The host key is "nats-service-host": rootCmd already // owns "nats-host" for its cluster-wide override, so reusing it would clobber. func bindNatsCollisionEnv() { - viper.BindEnv("nats-port", "SPINIFEX_NATS_PORT") - viper.BindPFlag("nats-port", natsCmd.PersistentFlags().Lookup("port")) + mustBindEnv("nats-port", "SPINIFEX_NATS_PORT") + mustBindPFlag("nats-port", natsCmd.PersistentFlags(), "port") - viper.BindEnv("nats-service-host", "SPINIFEX_NATS_HOST") - viper.BindPFlag("nats-service-host", natsCmd.PersistentFlags().Lookup("host")) + mustBindEnv("nats-service-host", "SPINIFEX_NATS_HOST") + mustBindPFlag("nats-service-host", natsCmd.PersistentFlags(), "host") - viper.BindEnv("nats-debug", "SPINIFEX_NATS_DEBUG") - viper.BindPFlag("nats-debug", natsCmd.PersistentFlags().Lookup("debug")) + mustBindEnv("nats-debug", "SPINIFEX_NATS_DEBUG") + mustBindPFlag("nats-debug", natsCmd.PersistentFlags(), "debug") } // bindAwsgwCollisionEnv namespaces awsgw's host, tls-cert, tls-key and debug // viper keys, which predastore and/or nats also bind bare. Each // AutomaticEnv-derived name now matches its own explicit BindEnv target. func bindAwsgwCollisionEnv() { - viper.BindEnv("awsgw-host", "SPINIFEX_AWSGW_HOST") - viper.BindPFlag("awsgw-host", awsgwCmd.PersistentFlags().Lookup("host")) + mustBindEnv("awsgw-host", "SPINIFEX_AWSGW_HOST") + mustBindPFlag("awsgw-host", awsgwCmd.PersistentFlags(), "host") - viper.BindEnv("awsgw-tls-cert", "SPINIFEX_AWSGW_TLS_CERT") - viper.BindPFlag("awsgw-tls-cert", awsgwCmd.PersistentFlags().Lookup("tls-cert")) + mustBindEnv("awsgw-tls-cert", "SPINIFEX_AWSGW_TLS_CERT") + mustBindPFlag("awsgw-tls-cert", awsgwCmd.PersistentFlags(), "tls-cert") - viper.BindEnv("awsgw-tls-key", "SPINIFEX_AWSGW_TLS_KEY") - viper.BindPFlag("awsgw-tls-key", awsgwCmd.PersistentFlags().Lookup("tls-key")) + mustBindEnv("awsgw-tls-key", "SPINIFEX_AWSGW_TLS_KEY") + mustBindPFlag("awsgw-tls-key", awsgwCmd.PersistentFlags(), "tls-key") - viper.BindEnv("awsgw-debug", "SPINIFEX_AWSGW_DEBUG") - viper.BindPFlag("awsgw-debug", awsgwCmd.PersistentFlags().Lookup("debug")) + mustBindEnv("awsgw-debug", "SPINIFEX_AWSGW_DEBUG") + mustBindPFlag("awsgw-debug", awsgwCmd.PersistentFlags(), "debug") } // bindViperblockEnv binds viperblock's S3 and plugin flags. The lookups must // target viperblockCmd, which declares them; a predastoreCmd lookup yields nil // and BindPFlag drops it silently, hiding both the flag and its default. func bindViperblockEnv() { - viper.BindEnv("s3-host", "SPINIFEX_VIPERBLOCK_S3_HOST") - viper.BindPFlag("s3-host", viperblockCmd.PersistentFlags().Lookup("s3-host")) + mustBindEnv("s3-host", "SPINIFEX_VIPERBLOCK_S3_HOST") + mustBindPFlag("s3-host", viperblockCmd.PersistentFlags(), "s3-host") - viper.BindEnv("s3-bucket", "SPINIFEX_VIPERBLOCK_S3_BUCKET") - viper.BindPFlag("s3-bucket", viperblockCmd.PersistentFlags().Lookup("s3-bucket")) + mustBindEnv("s3-bucket", "SPINIFEX_VIPERBLOCK_S3_BUCKET") + mustBindPFlag("s3-bucket", viperblockCmd.PersistentFlags(), "s3-bucket") - viper.BindEnv("s3-region", "SPINIFEX_VIPERBLOCK_S3_REGION") - viper.BindPFlag("s3-region", viperblockCmd.PersistentFlags().Lookup("s3-region")) + mustBindEnv("s3-region", "SPINIFEX_VIPERBLOCK_S3_REGION") + mustBindPFlag("s3-region", viperblockCmd.PersistentFlags(), "s3-region") - viper.BindEnv("plugin-path", "SPINIFEX_VIPERBLOCK_PLUGIN_PATH") - viper.BindPFlag("plugin-path", viperblockCmd.PersistentFlags().Lookup("plugin-path")) + mustBindEnv("plugin-path", "SPINIFEX_VIPERBLOCK_PLUGIN_PATH") + mustBindPFlag("plugin-path", viperblockCmd.PersistentFlags(), "plugin-path") } func init() { @@ -1123,20 +1123,20 @@ func init() { // key from predastore's encryption-key-file so the two BindPFlag calls // don't collide globally. viperblockCmd.PersistentFlags().String("encryption-key-file", "", "Path to the 32-byte AES-256 master key file for at-rest encryption (empty disables encryption)") - viper.BindEnv("viperblock-encryption-key-file", "SPINIFEX_VIPERBLOCK_ENCRYPTION_KEY_FILE") - viper.BindPFlag("viperblock-encryption-key-file", viperblockCmd.PersistentFlags().Lookup("encryption-key-file")) + mustBindEnv("viperblock-encryption-key-file", "SPINIFEX_VIPERBLOCK_ENCRYPTION_KEY_FILE") + mustBindPFlag("viperblock-encryption-key-file", viperblockCmd.PersistentFlags(), "encryption-key-file") viperblockCmd.PersistentFlags().Bool("debug", false, "Viperblock (EBS) debug logging") - viper.BindEnv("viperblock-debug", "SPINIFEX_VIPERBLOCK_DEBUG") - viper.BindPFlag("viperblock-debug", viperblockCmd.PersistentFlags().Lookup("debug")) + mustBindEnv("viperblock-debug", "SPINIFEX_VIPERBLOCK_DEBUG") + mustBindPFlag("viperblock-debug", viperblockCmd.PersistentFlags(), "debug") viperblockCmd.AddCommand(viperblockStartCmd) serviceCmd.AddCommand(qemunbdCmd) qemunbdCmd.PersistentFlags().Bool("debug", false, "qemunbd (EBS) debug logging") - viper.BindEnv("qemunbd-debug", "SPINIFEX_QEMUNBD_DEBUG") - viper.BindPFlag("qemunbd-debug", qemunbdCmd.PersistentFlags().Lookup("debug")) + mustBindEnv("qemunbd-debug", "SPINIFEX_QEMUNBD_DEBUG") + mustBindPFlag("qemunbd-debug", qemunbdCmd.PersistentFlags(), "debug") qemunbdCmd.AddCommand(qemunbdStartCmd) @@ -1154,12 +1154,12 @@ func init() { bindNatsCollisionEnv() natsCmd.PersistentFlags().String("data-dir", "", "NATS data directory") - viper.BindEnv("data-dir", "SPINIFEX_NATS_DATA_DIR") - viper.BindPFlag("data-dir", natsCmd.PersistentFlags().Lookup("data-dir")) + mustBindEnv("data-dir", "SPINIFEX_NATS_DATA_DIR") + mustBindPFlag("data-dir", natsCmd.PersistentFlags(), "data-dir") natsCmd.PersistentFlags().Bool("jetstream", false, "Enable JetStream") - viper.BindEnv("jetstream", "SPINIFEX_NATS_JETSTREAM") - viper.BindPFlag("jetstream", natsCmd.PersistentFlags().Lookup("jetstream")) + mustBindEnv("jetstream", "SPINIFEX_NATS_JETSTREAM") + mustBindPFlag("jetstream", natsCmd.PersistentFlags(), "jetstream") // Spinifex serviceCmd.AddCommand(spinifexCmd) @@ -1167,8 +1167,8 @@ func init() { spinifexCmd.AddCommand(spinifexStartCmd) spinifexCmd.PersistentFlags().String("wal-dir", "", "Write-ahead log (WAL) directory. Place on high-speed NVMe disk, or tmpfs for development.") - viper.BindEnv("wal-dir", "SPINIFEX_WAL_DIR") - viper.BindPFlag("wal-dir", spinifexCmd.PersistentFlags().Lookup("wal-dir")) + mustBindEnv("wal-dir", "SPINIFEX_WAL_DIR") + mustBindPFlag("wal-dir", spinifexCmd.PersistentFlags(), "wal-dir") // AWS GW serviceCmd.AddCommand(awsgwCmd) @@ -1192,28 +1192,28 @@ func init() { serviceCmd.AddCommand(spinifexUICmd) spinifexUICmd.PersistentFlags().Int("port", 3000, "spinifex-ui server port") - viper.BindEnv("spinifex-ui-port", "SPINIFEX_UI_PORT") - viper.BindPFlag("spinifex-ui-port", spinifexUICmd.PersistentFlags().Lookup("port")) + mustBindEnv("spinifex-ui-port", "SPINIFEX_UI_PORT") + mustBindPFlag("spinifex-ui-port", spinifexUICmd.PersistentFlags(), "port") spinifexUICmd.PersistentFlags().String("host", "0.0.0.0", "spinifex-ui server host") - viper.BindEnv("spinifex-ui-host", "SPINIFEX_UI_HOST") - viper.BindPFlag("spinifex-ui-host", spinifexUICmd.PersistentFlags().Lookup("host")) + mustBindEnv("spinifex-ui-host", "SPINIFEX_UI_HOST") + mustBindPFlag("spinifex-ui-host", spinifexUICmd.PersistentFlags(), "host") spinifexUICmd.PersistentFlags().String("tls-cert", "", "TLS certificate path") - viper.BindEnv("spinifex-ui-tls-cert", "SPINIFEX_UI_TLS_CERT") - viper.BindPFlag("spinifex-ui-tls-cert", spinifexUICmd.PersistentFlags().Lookup("tls-cert")) + mustBindEnv("spinifex-ui-tls-cert", "SPINIFEX_UI_TLS_CERT") + mustBindPFlag("spinifex-ui-tls-cert", spinifexUICmd.PersistentFlags(), "tls-cert") spinifexUICmd.PersistentFlags().String("tls-key", "", "TLS key path") - viper.BindEnv("spinifex-ui-tls-key", "SPINIFEX_UI_TLS_KEY") - viper.BindPFlag("spinifex-ui-tls-key", spinifexUICmd.PersistentFlags().Lookup("tls-key")) + mustBindEnv("spinifex-ui-tls-key", "SPINIFEX_UI_TLS_KEY") + mustBindPFlag("spinifex-ui-tls-key", spinifexUICmd.PersistentFlags(), "tls-key") spinifexUICmd.PersistentFlags().String("base-dir", "", "spinifex-ui base directory for PID files and state") - viper.BindEnv("spinifex-ui-base-dir", "SPINIFEX_UI_BASE_DIR") - viper.BindPFlag("spinifex-ui-base-dir", spinifexUICmd.PersistentFlags().Lookup("base-dir")) + mustBindEnv("spinifex-ui-base-dir", "SPINIFEX_UI_BASE_DIR") + mustBindPFlag("spinifex-ui-base-dir", spinifexUICmd.PersistentFlags(), "base-dir") spinifexUICmd.PersistentFlags().Bool("ochre-enabled", false, "enable the Ochre console") - viper.BindEnv("spinifex-ui-ochre-enabled", "SPINIFEX_UI_OCHRE_ENABLED") - viper.BindPFlag("spinifex-ui-ochre-enabled", spinifexUICmd.PersistentFlags().Lookup("ochre-enabled")) + mustBindEnv("spinifex-ui-ochre-enabled", "SPINIFEX_UI_OCHRE_ENABLED") + mustBindPFlag("spinifex-ui-ochre-enabled", spinifexUICmd.PersistentFlags(), "ochre-enabled") spinifexUICmd.AddCommand(spinifexUIStartCmd) @@ -1226,8 +1226,8 @@ func init() { serviceCmd.AddCommand(northstarCmd) northstarCmd.PersistentFlags().String("northstar-config", "", "Path to northstar.toml (overrides config file)") - viper.BindEnv("northstar-config", "SPINIFEX_NORTHSTAR_CONFIG") - viper.BindPFlag("northstar-config", northstarCmd.PersistentFlags().Lookup("northstar-config")) + mustBindEnv("northstar-config", "SPINIFEX_NORTHSTAR_CONFIG") + mustBindPFlag("northstar-config", northstarCmd.PersistentFlags(), "northstar-config") northstarCmd.AddCommand(northstarStartCmd) diff --git a/cmd/spinifex/cmd/service_bind_collision_test.go b/spinifex/operator/cli/service_bind_collision_test.go similarity index 99% rename from cmd/spinifex/cmd/service_bind_collision_test.go rename to spinifex/operator/cli/service_bind_collision_test.go index f00491b44..ff8ed1182 100644 --- a/cmd/spinifex/cmd/service_bind_collision_test.go +++ b/spinifex/operator/cli/service_bind_collision_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "strings" diff --git a/cmd/spinifex/cmd/service_console_region_test.go b/spinifex/operator/cli/service_console_region_test.go similarity index 80% rename from cmd/spinifex/cmd/service_console_region_test.go rename to spinifex/operator/cli/service_console_region_test.go index b59518b00..0843caf14 100644 --- a/cmd/spinifex/cmd/service_console_region_test.go +++ b/spinifex/operator/cli/service_console_region_test.go @@ -1,10 +1,10 @@ -package cmd_test +package cli_test import ( "testing" - "github.com/mulgadc/spinifex/cmd/spinifex/cmd" "github.com/mulgadc/spinifex/spinifex/bootstrap/config" + "github.com/mulgadc/spinifex/spinifex/operator/cli" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,7 +18,7 @@ func TestConsoleRegion_ReadsTheServingNode(t *testing.T) { }, } - region, err := cmd.ConsoleRegion(cfg) + region, err := cli.ConsoleRegion(cfg) require.NoError(t, err) assert.Equal(t, "us-west-1", region) @@ -31,14 +31,14 @@ func TestConsoleRegion_FailsWhenNodeHasNoRegion(t *testing.T) { Nodes: map[string]config.Config{"node1": {}}, } - _, err := cmd.ConsoleRegion(cfg) + _, err := cli.ConsoleRegion(cfg) require.Error(t, err) assert.Contains(t, err.Error(), "node1") } func TestConsoleRegion_FailsWhenNodeIsAbsent(t *testing.T) { - _, err := cmd.ConsoleRegion(&config.ClusterConfig{Node: "ghost"}) + _, err := cli.ConsoleRegion(&config.ClusterConfig{Node: "ghost"}) require.Error(t, err) } diff --git a/cmd/spinifex/cmd/service_exit_test.go b/spinifex/operator/cli/service_exit_test.go similarity index 99% rename from cmd/spinifex/cmd/service_exit_test.go rename to spinifex/operator/cli/service_exit_test.go index 6d75de778..0c89ae4f3 100644 --- a/cmd/spinifex/cmd/service_exit_test.go +++ b/spinifex/operator/cli/service_exit_test.go @@ -1,4 +1,4 @@ -package cmd +package cli // Drives the unexported *cobra.Command vars (predastoreStartCmd, etc.) // directly via RunE, so it must stay in-package. diff --git a/cmd/spinifex/cmd/service_test.go b/spinifex/operator/cli/service_test.go similarity index 99% rename from cmd/spinifex/cmd/service_test.go rename to spinifex/operator/cli/service_test.go index 1beb5c4a4..2bd7e85f4 100644 --- a/cmd/spinifex/cmd/service_test.go +++ b/spinifex/operator/cli/service_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "errors" diff --git a/cmd/spinifex/cmd/telemetry_test.go b/spinifex/operator/cli/telemetry_test.go similarity index 97% rename from cmd/spinifex/cmd/telemetry_test.go rename to spinifex/operator/cli/telemetry_test.go index b85a0b72b..b009396e0 100644 --- a/cmd/spinifex/cmd/telemetry_test.go +++ b/spinifex/operator/cli/telemetry_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import "testing" diff --git a/cmd/spinifex/cmd/templates/awsgw.toml b/spinifex/operator/cli/templates/awsgw.toml similarity index 100% rename from cmd/spinifex/cmd/templates/awsgw.toml rename to spinifex/operator/cli/templates/awsgw.toml diff --git a/cmd/spinifex/cmd/templates/nats.conf b/spinifex/operator/cli/templates/nats.conf similarity index 100% rename from cmd/spinifex/cmd/templates/nats.conf rename to spinifex/operator/cli/templates/nats.conf diff --git a/cmd/spinifex/cmd/templates/northstar.toml b/spinifex/operator/cli/templates/northstar.toml similarity index 100% rename from cmd/spinifex/cmd/templates/northstar.toml rename to spinifex/operator/cli/templates/northstar.toml diff --git a/cmd/spinifex/cmd/templates/predastore-multinode.toml b/spinifex/operator/cli/templates/predastore-multinode.toml similarity index 100% rename from cmd/spinifex/cmd/templates/predastore-multinode.toml rename to spinifex/operator/cli/templates/predastore-multinode.toml diff --git a/cmd/spinifex/cmd/templates/predastore.toml b/spinifex/operator/cli/templates/predastore.toml similarity index 100% rename from cmd/spinifex/cmd/templates/predastore.toml rename to spinifex/operator/cli/templates/predastore.toml diff --git a/cmd/spinifex/cmd/templates/spinifex.toml b/spinifex/operator/cli/templates/spinifex.toml similarity index 100% rename from cmd/spinifex/cmd/templates/spinifex.toml rename to spinifex/operator/cli/templates/spinifex.toml diff --git a/cmd/spinifex/cmd/top.go b/spinifex/operator/cli/top.go similarity index 83% rename from cmd/spinifex/cmd/top.go rename to spinifex/operator/cli/top.go index c77bda198..5f47e0b17 100644 --- a/cmd/spinifex/cmd/top.go +++ b/spinifex/operator/cli/top.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "encoding/json" @@ -113,7 +113,15 @@ func runTopNodes(cmd *cobra.Command, args []string) { } } - pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(nodeTable).Render() + // Render discards its write error, so write the table here to fail on it. + rendered, err := pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(nodeTable).Srender() + if err == nil { + _, err = fmt.Fprintln(os.Stdout, rendered) + } + if err != nil { + fmt.Fprintf(os.Stderr, "Error: render table: %v\n", err) + os.Exit(1) + } // Instance type capacity summary if len(capacityMap) == 0 { @@ -138,7 +146,15 @@ func runTopNodes(cmd *cobra.Command, args []string) { }) } - pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(capTable).Render() + // Render discards its write error, so write the table here to fail on it. + rendered, err = pterm.DefaultTable.WithHasHeader().WithLeftAlignment().WithData(capTable).Srender() + if err == nil { + _, err = fmt.Fprintln(os.Stdout, rendered) + } + if err != nil { + fmt.Fprintf(os.Stderr, "Error: render table: %v\n", err) + os.Exit(1) + } } type aggregatedCap struct { diff --git a/cmd/spinifex/cmd/version.go b/spinifex/operator/cli/version.go similarity index 79% rename from cmd/spinifex/cmd/version.go rename to spinifex/operator/cli/version.go index ca1cd5495..5c5fbc585 100644 --- a/cmd/spinifex/cmd/version.go +++ b/spinifex/operator/cli/version.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "fmt" @@ -8,7 +8,7 @@ import ( ) // Version variables set via ldflags at build time. -// Example: go build -ldflags "-X github.com/mulgadc/spinifex/cmd/spinifex/cmd.Version=v1.0.0". +// Example: go build -ldflags "-X github.com/mulgadc/spinifex/spinifex/operator/cli.Version=v1.0.0". var ( Version = "dev" Commit = "unknown" diff --git a/cmd/spinifex/cmd/viperblock_region_test.go b/spinifex/operator/cli/viperblock_region_test.go similarity index 99% rename from cmd/spinifex/cmd/viperblock_region_test.go rename to spinifex/operator/cli/viperblock_region_test.go index 559ab531d..e80874766 100644 --- a/cmd/spinifex/cmd/viperblock_region_test.go +++ b/spinifex/operator/cli/viperblock_region_test.go @@ -1,4 +1,4 @@ -package cmd +package cli import ( "testing" diff --git a/spinifex/operator/cli/wiring.go b/spinifex/operator/cli/wiring.go new file mode 100644 index 000000000..f0418f1f7 --- /dev/null +++ b/spinifex/operator/cli/wiring.go @@ -0,0 +1,33 @@ +package cli + +import ( + "fmt" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/spf13/viper" +) + +// mustBindEnv binds a config key to its environment variables. A wiring error +// would otherwise leave the key silently unbound, so it panics at startup. +func mustBindEnv(key string, envVars ...string) { + if err := viper.BindEnv(append([]string{key}, envVars...)...); err != nil { + panic(fmt.Sprintf("bind config key %q to env %v: %v", key, envVars, err)) //nolint:forbidigo // Startup wiring error: fail loudly rather than run with a silently unbound flag or config key. + } +} + +// mustBindPFlag binds a config key to a flag. Looking the flag up on a command +// that does not declare it yields nil, which viper would otherwise drop silently. +func mustBindPFlag(key string, flags *pflag.FlagSet, name string) { + if err := viper.BindPFlag(key, flags.Lookup(name)); err != nil { + panic(fmt.Sprintf("bind config key %q to flag %q: %v", key, name, err)) //nolint:forbidigo // Startup wiring error: fail loudly rather than run with a silently unbound flag or config key. + } +} + +// mustMarkFlagRequired marks a flag required, panicking when the command does +// not declare it rather than silently dropping the required check. +func mustMarkFlagRequired(cmd *cobra.Command, name string) { + if err := cmd.MarkFlagRequired(name); err != nil { + panic(fmt.Sprintf("mark flag %q required on %q: %v", name, cmd.Name(), err)) //nolint:forbidigo // Startup wiring error: fail loudly rather than run with a silently unbound flag or config key. + } +} diff --git a/spinifex/operator/cli/wiring_test.go b/spinifex/operator/cli/wiring_test.go new file mode 100644 index 000000000..c22aec63e --- /dev/null +++ b/spinifex/operator/cli/wiring_test.go @@ -0,0 +1,24 @@ +package cli + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" +) + +func TestMustBindPFlag_PanicsNamingKeyAndFlagWhenUndeclared(t *testing.T) { + flags := pflag.NewFlagSet("declares-other", pflag.ContinueOnError) + flags.String("other", "", "") + assert.PanicsWithValue(t, `bind config key "s3-host" to flag "s3-host": flag for "s3-host" is nil`, func() { + mustBindPFlag("s3-host", flags, "s3-host") + }) +} + +func TestMustMarkFlagRequired_PanicsNamingFlagAndCommandWhenUndeclared(t *testing.T) { + c := &cobra.Command{Use: "probe"} + assert.PanicsWithValue(t, `mark flag "model-id" required on "probe": no such flag -model-id`, func() { + mustMarkFlagRequired(c, "model-id") + }) +} diff --git a/spinifex/runtime/roles/nats/nats_test.go b/spinifex/runtime/roles/nats/nats_test.go index 9501dc87b..7a4104aa2 100644 --- a/spinifex/runtime/roles/nats/nats_test.go +++ b/spinifex/runtime/roles/nats/nats_test.go @@ -23,7 +23,7 @@ func templatePath(t *testing.T) string { t.Helper() _, thisFile, _, ok := runtime.Caller(0) require.True(t, ok) - return filepath.Join(filepath.Dir(thisFile), "..", "..", "..", "..", "cmd", "spinifex", "cmd", "templates", "nats.conf") + return filepath.Join(filepath.Dir(thisFile), "..", "..", "..", "..", "spinifex", "operator", "cli", "templates", "nats.conf") } // TestRenderedConfig_EnforcesAuth renders the production nats.conf template,