gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-15 17:11:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: stop viper AutomaticEnv from shadowing predastore env vars (#659)

Browse Source 
* fix: stop viper AutomaticEnv from shadowing predastore's own env vars

Viper resolves AutomaticEnv before explicit BindEnv, so predastore's
bare "config-path", "host" and "base-path" viper keys auto-derived to
SPINIFEX_CONFIG_PATH, SPINIFEX_HOST and SPINIFEX_BASE_PATH -
coincidentally the same names used elsewhere (the global cluster
config path, and root's global --host override). Setting
SPINIFEX_CONFIG_PATH for the cluster config (needed by
derivePredastoreBind) silently made predastore parse the cluster
config as its own S3 config, breaking auth for every signed request.

Rename predastore's host/base-path/config-path viper keys to
"predastore-"-prefixed keys whose AutomaticEnv-derived name is
identical to their explicit BindEnv target, so no shadow is possible
regardless of what else is set in the environment.

* style: trim predastore env-binding comments to the 3-line limit
This commit is contained in:
Josh Siviour
2026-08-03 00:03:28 +10:00
committed by GitHub
parent 5de598e85a
commit bbf1ca7f0b
2 changed files with 54 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/spinifex/cmd/predastore_bind_test.go
+23
View File
@@ -3,6 +3,7 @@ package cmd
import (
"os"
"path/filepath"
"strings"
"testing"
"github.com/mulgadc/spinifex/spinifex/config"
@@ -136,3 +137,25 @@ func TestDerivePredastoreBind_MissingHostErrors(t *testing.T) {
_, err := derivePredastoreBind(clusterConfig)
require.Error(t, err)
}
// A bare "config-path" key derives the same AutomaticEnv name as the
// unrelated cluster-config SPINIFEX_CONFIG_PATH, which made predastore load
// the cluster config as its own S3 config.
func TestPredastoreConfigPathNotShadowedByClusterConfigPathEnv(t *testing.T) {
t.Cleanup(func() { viper.Reset() })
viper.Reset()
// Mirrors service.go's init() against the freshly-Reset instance. pflags
// survive Reset: they live on predastoreCmd, a package-level singleton.
viper.SetEnvPrefix("SPINIFEX")
viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
viper.AutomaticEnv()
bindPredastoreNamespacedEnv()
t.Setenv("SPINIFEX_CONFIG_PATH", "/etc/spinifex/spinifex.toml")
t.Setenv("SPINIFEX_PREDASTORE_CONFIG_PATH", "/etc/spinifex/predastore/predastore.toml")
got := viper.GetString("predastore-config-path")
assert.Equal(t, "/etc/spinifex/predastore/predastore.toml", got,
"predastore-config-path must resolve to the predastore config even when SPINIFEX_CONFIG_PATH is also set")
}
cmd/spinifex/cmd/service.go
+31 -18
View File
@@ -141,8 +141,8 @@ var predastoreStartCmd = &cobra.Command{
// Get the port from the flags
port := viper.GetInt("port")
host := viper.GetString("host")
basePath := viper.GetString("base-path")
host := viper.GetString("predastore-host")
basePath := viper.GetString("predastore-base-path")
debug := viper.GetBool("debug")
nodeID := viper.GetInt("node-id")
@@ -160,7 +160,7 @@ var predastoreStartCmd = &cobra.Command{
fmt.Println("Error deriving predastore bind config:", err)
return
}
if !viper.IsSet("host") {
if !viper.IsSet("predastore-host") {
host = bind.Host
}
if !viper.IsSet("port") {
@@ -177,7 +177,7 @@ var predastoreStartCmd = &cobra.Command{
return
}
configPath := viper.GetString("config-path")
configPath := viper.GetString("predastore-config-path")
if configPath == "" {
fmt.Println("Config path is not set")
@@ -1157,6 +1157,30 @@ var qmpCollectorStatusCmd = &cobra.Command{
},
}
// registerPredastoreNamespacedFlags defines predastore's host, base-path and
// config-path flags and binds them to viper. It must only run once (from
// init()); pflag panics on a redefined flag.
func registerPredastoreNamespacedFlags() {
predastoreCmd.PersistentFlags().String("host", "0.0.0.0", "Predastore (S3) host")
predastoreCmd.PersistentFlags().String("base-path", "", "Predastore (S3) base path")
predastoreCmd.PersistentFlags().String("config-path", "", "Predastore (S3) config path")
bindPredastoreNamespacedEnv()
}
// bindPredastoreNamespacedEnv uses "predastore-"-prefixed viper keys so each
// 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"))
viper.BindEnv("predastore-base-path", "SPINIFEX_PREDASTORE_BASE_PATH")
viper.BindPFlag("predastore-base-path", predastoreCmd.PersistentFlags().Lookup("base-path"))
viper.BindEnv("predastore-config-path", "SPINIFEX_PREDASTORE_CONFIG_PATH")
viper.BindPFlag("predastore-config-path", predastoreCmd.PersistentFlags().Lookup("config-path"))
}
func init() {
viper.SetEnvPrefix("SPINIFEX") // Prefix for environment variables
viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
@@ -1172,20 +1196,9 @@ func init() {
viper.BindEnv("port", "SPINIFEX_PREDASTORE_PORT")
viper.BindPFlag("port", predastoreCmd.PersistentFlags().Lookup("port"))
// Predastore Host
predastoreCmd.PersistentFlags().String("host", "0.0.0.0", "Predastore (S3) host")
viper.BindEnv("host", "SPINIFEX_PREDASTORE_HOST")
viper.BindPFlag("host", predastoreCmd.PersistentFlags().Lookup("host"))
// Base path
predastoreCmd.PersistentFlags().String("base-path", "", "Predastore (S3) base path")
viper.BindEnv("base-path", "SPINIFEX_PREDASTORE_BASE_PATH")
viper.BindPFlag("base-path", predastoreCmd.PersistentFlags().Lookup("base-path"))
// Predastore Config Path
predastoreCmd.PersistentFlags().String("config-path", "", "Predastore (S3) config path")
viper.BindEnv("config-path", "SPINIFEX_PREDASTORE_CONFIG_PATH")
viper.BindPFlag("config-path", predastoreCmd.PersistentFlags().Lookup("config-path"))
// Predastore host, base-path, config-path (namespaced viper keys —
// see registerPredastoreNamespacedFlags doc comment)
registerPredastoreNamespacedFlags()
// Predastore Debug
predastoreCmd.PersistentFlags().Bool("debug", false, "Predastore (S3) debug")