mirror of
https://github.com/safedep/pmg.git
synced 2026-08-03 07:24:09 +02:00
fix: Support PMG_* env vars regardless of config file state
AutomaticEnv only resolves env vars for keys Viper already knows about via AllKeys(). When a key is absent from the config file (commented out, new key added after last setup, or no config file at all), Viper had no knowledge of it and silently skipped the env var. Fix by registering all Config struct fields as Viper defaults via reflection (using mapstructure tags) before reading the config file. This ensures PMG_* env vars work in all cases. Precedence: cobra flags > env vars > config file > defaults. SetDefault is used (not Set) so env vars and config file can still override the Go defaults freely. Tests added covering all precedence levels including the key-absent- from-config-file case that was the original bug report.
This commit is contained in:
Sahilb315
@@ -5,6 +5,7 @@ import (
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/cobra"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -124,6 +125,89 @@ func TestProxyInstallOnlyConfig(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// TestConfigPrecedence verifies the expected override order:
|
||||
// flags > env var > config file > default
|
||||
func TestConfigPrecedence(t *testing.T) {
|
||||
t.Run("env var overrides config file", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
t.Setenv("PMG_CONFIG_DIR", tmpDir)
|
||||
t.Setenv("PMG_PROXY_INSTALL_ONLY", "true")
|
||||
|
||||
configPath := filepath.Join(tmpDir, "config.yml")
|
||||
err := os.WriteFile(configPath, []byte("proxy_install_only: false\n"), 0o644)
|
||||
require.NoError(t, err)
|
||||
|
||||
initConfig()
|
||||
assert.Equal(t, true, Get().Config.ProxyInstallOnly, "env var should override config file")
|
||||
})
|
||||
|
||||
t.Run("config file overrides default", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
t.Setenv("PMG_CONFIG_DIR", tmpDir)
|
||||
t.Setenv("PMG_PROXY_INSTALL_ONLY", "")
|
||||
|
||||
configPath := filepath.Join(tmpDir, "config.yml")
|
||||
err := os.WriteFile(configPath, []byte("proxy_install_only: true\n"), 0o644)
|
||||
require.NoError(t, err)
|
||||
|
||||
initConfig()
|
||||
assert.Equal(t, true, Get().Config.ProxyInstallOnly, "config file should override default")
|
||||
})
|
||||
|
||||
t.Run("env var works when key is absent from config file", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
t.Setenv("PMG_CONFIG_DIR", tmpDir)
|
||||
t.Setenv("PMG_PROXY_INSTALL_ONLY", "true")
|
||||
|
||||
// Config file exists but proxy_install_only is not in it (e.g. commented out
|
||||
// or user hasn't re-run "pmg setup install" after an upgrade)
|
||||
configPath := filepath.Join(tmpDir, "config.yml")
|
||||
err := os.WriteFile(configPath, []byte("transitive: false\n"), 0o644)
|
||||
require.NoError(t, err)
|
||||
|
||||
initConfig()
|
||||
assert.Equal(t, true, Get().Config.ProxyInstallOnly, "env var should work even when key is absent from config file")
|
||||
})
|
||||
|
||||
t.Run("env var works without a config file", func(t *testing.T) {
|
||||
t.Setenv("PMG_CONFIG_DIR", "/tmp/pmg-test/random-does-not-exist")
|
||||
t.Setenv("PMG_PROXY_INSTALL_ONLY", "true")
|
||||
|
||||
initConfig()
|
||||
assert.Equal(t, true, Get().Config.ProxyInstallOnly, "env var should work even without a config file")
|
||||
})
|
||||
|
||||
t.Run("default is used when neither env var nor config file sets the key", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
t.Setenv("PMG_CONFIG_DIR", tmpDir)
|
||||
t.Setenv("PMG_PROXY_INSTALL_ONLY", "")
|
||||
|
||||
configPath := filepath.Join(tmpDir, "config.yml")
|
||||
err := os.WriteFile(configPath, []byte("transitive: false\n"), 0o644)
|
||||
require.NoError(t, err)
|
||||
|
||||
initConfig()
|
||||
assert.Equal(t, false, Get().Config.ProxyInstallOnly, "should use default when key absent from config and env")
|
||||
})
|
||||
|
||||
t.Run("cobra flag overrides env var", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
t.Setenv("PMG_CONFIG_DIR", tmpDir)
|
||||
t.Setenv("PMG_PARANOID", "true")
|
||||
|
||||
initConfig()
|
||||
assert.Equal(t, true, Get().Config.Paranoid, "env var should set paranoid=true")
|
||||
|
||||
// Simulate cobra flag parsing — BoolVar writes directly to the struct
|
||||
// field after Viper, giving flags the highest effective precedence.
|
||||
cmd := &cobra.Command{}
|
||||
ApplyCobraFlags(cmd)
|
||||
require.NoError(t, cmd.ParseFlags([]string{"--paranoid=false"}))
|
||||
|
||||
assert.Equal(t, false, Get().Config.Paranoid, "cobra flag should override env var")
|
||||
})
|
||||
}
|
||||
|
||||
func TestWriteTemplateConfigMergesExistingConfig(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
t.Setenv("PMG_CONFIG_DIR", tmpDir)
|
||||
|
||||
+57
-22
@@ -3,47 +3,43 @@ package config
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"reflect"
|
||||
"strings"
|
||||
|
||||
"github.com/spf13/viper"
|
||||
)
|
||||
|
||||
// loadViperConfig loads the configuration using Viper if available.
|
||||
// It returns an error if the config file exists but cannot be read or parsed,
|
||||
// allowing the caller to fall back to default configuration.
|
||||
// loadViperConfig loads the configuration using Viper.
|
||||
// Precedence (highest to lowest): cobra flags > env vars > config file > defaults.
|
||||
// Cobra flags write directly to the config struct after this function runs.
|
||||
func loadViperConfig() error {
|
||||
configPath, err := configFilePath()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to get config file path: %w", err)
|
||||
}
|
||||
|
||||
// Check if config file exists before attempting to load
|
||||
// If it doesn't exist, we use the default configuration (see config.go)
|
||||
if _, err := os.Stat(configPath); os.IsNotExist(err) {
|
||||
return nil
|
||||
}
|
||||
|
||||
v := viper.New()
|
||||
|
||||
v.SetConfigFile(configPath)
|
||||
v.SetConfigType("yaml")
|
||||
v.SetEnvPrefix("PMG")
|
||||
v.AutomaticEnv()
|
||||
v.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
|
||||
|
||||
if err := v.ReadInConfig(); err != nil {
|
||||
return fmt.Errorf("failed to read config file %s: %w", configPath, err)
|
||||
// Register all config struct fields as Viper defaults so that env vars work
|
||||
// for any key Viper wouldn't otherwise know about — either because there is no
|
||||
// config file, or because the key is absent from the file (e.g. commented out,
|
||||
// or a new key added after the user last ran "pmg setup install").
|
||||
registerViperDefaults(v, globalConfig.Config, "")
|
||||
|
||||
// Merge the user config file on top if it exists.
|
||||
if _, statErr := os.Stat(configPath); statErr == nil {
|
||||
v.SetConfigFile(configPath)
|
||||
if err := v.MergeInConfig(); err != nil {
|
||||
return fmt.Errorf("failed to read config file %s: %w", configPath, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Unmarshal into a copy of the current defaults (not a zero-value struct) so that
|
||||
// keys missing from the YAML retain their defaults. This is critical for users who
|
||||
// upgrade PMG without re-running "pmg setup install" — their old config.yml won't have
|
||||
// newer keys (e.g. dependency_cooldown), and those must fall back to defaults rather than
|
||||
// silently becoming Go zero values (false/0/"").
|
||||
//
|
||||
// We use a copy rather than unmarshalling directly into globalConfig.Config so that
|
||||
// on error the caller's "using defaults" fallback is truthful — globalConfig.Config
|
||||
// stays in a clean default state instead of being partially overwritten.
|
||||
// Unmarshal into a copy of the current defaults so that keys absent from
|
||||
// both the env and the user config file retain their Go defaults.
|
||||
merged := globalConfig.Config
|
||||
if err := v.Unmarshal(&merged); err != nil {
|
||||
return fmt.Errorf("failed to unmarshal config: %w", err)
|
||||
@@ -52,3 +48,42 @@ func loadViperConfig() error {
|
||||
globalConfig.Config = merged
|
||||
return nil
|
||||
}
|
||||
|
||||
// registerViperDefaults walks cfg (a struct) recursively via reflection and registers
|
||||
// each field as a Viper default using its mapstructure tag as the key. This is the
|
||||
// minimum required for AutomaticEnv to resolve env vars for those keys.
|
||||
func registerViperDefaults(v *viper.Viper, cfg any, prefix string) {
|
||||
t := reflect.TypeOf(cfg)
|
||||
val := reflect.ValueOf(cfg)
|
||||
|
||||
if t.Kind() == reflect.Pointer {
|
||||
t = t.Elem()
|
||||
val = val.Elem()
|
||||
}
|
||||
|
||||
if t.Kind() != reflect.Struct {
|
||||
return
|
||||
}
|
||||
|
||||
for i := range t.NumField() {
|
||||
field := t.Field(i)
|
||||
fieldVal := val.Field(i)
|
||||
|
||||
tag := field.Tag.Get("mapstructure")
|
||||
if tag == "" || tag == "-" {
|
||||
continue
|
||||
}
|
||||
|
||||
// Strip options like ",squash" or ",omitempty"
|
||||
key := strings.SplitN(tag, ",", 2)[0]
|
||||
if prefix != "" {
|
||||
key = prefix + "." + key
|
||||
}
|
||||
|
||||
if field.Type.Kind() == reflect.Struct {
|
||||
registerViperDefaults(v, fieldVal.Interface(), key)
|
||||
} else {
|
||||
v.SetDefault(key, fieldVal.Interface())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user