synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-15 22:20:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Disable viper auto-env to avoid assigning to parent structs (#4893)

Browse Source 
The viper automatic env mapping and BindEnv were both creating mappings
like `vip.BindEnv("telemetry", "CODER_TELEMETRY")` which we don't want
since `DeploymentConfig.Telemetry` is a struct housing fields.

For some reason, this was causing `DeploymentConfig.Telemetry.URL` to
**not** be assigned its default value when `CODER_TELEMETRY=false` was
set as an environment variable.

Potentially we would want `"telemetry.enable"` to be mapped to
`"CODER_TELEMETRY"` for simplicity. But that behavior is not changed by
this commit.

Arguably, we could remove `vip.SetEnvPrefix` and `vip.SetEnvKeyReplacer`
as well since we're manually controlling all environment variable names
via `formatEnv`.
This commit is contained in:
Mathias Fredriksson
2022-11-04 19:46:59 +02:00
committed by GitHub
parent 1bbe37a602
commit 70048acd73
2 changed files with 28 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
cli/deployment/config.go
View File

@ -370,7 +370,6 @@ func Config(flagset *pflag.FlagSet, vip *viper.Viper) (*codersdk.DeploymentConfi
return nil, xerrors.Errorf("get global config from flag: %w", err)
}
vip.SetEnvPrefix("coder")
vip.AutomaticEnv()
if flg != "" {
vip.SetConfigFile(flg + "/server.yaml")
@ -393,21 +392,26 @@ func setConfig(prefix string, vip *viper.Viper, target interface{}) {
typ = val.Type()
}
// Manually bind to env to support CODER_$INDEX_$FIELD format for structured slices.
_ = vip.BindEnv(prefix, formatEnv(prefix))
// Ensure that we only bind env variables to proper fields,
// otherwise Viper will get confused if the parent struct is
// assigned a value.
if strings.HasPrefix(typ.Name(), "DeploymentConfigField[") {
value := val.FieldByName("Value").Interface()
switch value.(type) {
case string:
vip.MustBindEnv(prefix, formatEnv(prefix))
val.FieldByName("Value").SetString(vip.GetString(prefix))
case bool:
vip.MustBindEnv(prefix, formatEnv(prefix))
val.FieldByName("Value").SetBool(vip.GetBool(prefix))
case int:
vip.MustBindEnv(prefix, formatEnv(prefix))
val.FieldByName("Value").SetInt(int64(vip.GetInt(prefix)))
case time.Duration:
vip.MustBindEnv(prefix, formatEnv(prefix))
val.FieldByName("Value").SetInt(int64(vip.GetDuration(prefix)))
case []string:
vip.MustBindEnv(prefix, formatEnv(prefix))
// As of October 21st, 2022 we supported delimiting a string
// with a comma, but Viper only supports with a space. This
// is a small hack around it!
@ -422,6 +426,7 @@ func setConfig(prefix string, vip *viper.Viper, target interface{}) {
}
val.FieldByName("Value").Set(reflect.ValueOf(value))
case []codersdk.GitAuthConfig:
// Do not bind to CODER_GITAUTH, instead bind to CODER_GITAUTH_0_*, etc.
values := readSliceFromViper[codersdk.GitAuthConfig](vip, prefix, value)
val.FieldByName("Value").Set(reflect.ValueOf(values))
default:
@ -471,6 +476,11 @@ func readSliceFromViper[T any](vip *viper.Viper, key string, value any) []T {
prop = fve.Tag.Get("yaml")
}
configKey := fmt.Sprintf("%s.%d.%s", key, entry, prop)
// Ensure the env entry for this key is registered
// before checking value.
vip.MustBindEnv(configKey, formatEnv(configKey))
value := vip.Get(configKey)
if value == nil {
continue
@ -502,7 +512,6 @@ func NewViper() *viper.Viper {
dc := newConfig()
vip := viper.New()
vip.SetEnvPrefix("coder")
vip.AutomaticEnv()
vip.SetEnvKeyReplacer(strings.NewReplacer("-", "_", ".", "_"))
setViperDefaults("", vip, dc)
@ -566,6 +575,10 @@ func setFlags(prefix string, flagset *pflag.FlagSet, vip *viper.Viper, target in
hidden := val.FieldByName("Hidden").Bool()
value := val.FieldByName("Default").Interface()
// Allow currently set environment variables
// to override default values in help output.
vip.MustBindEnv(prefix, formatEnv(prefix))
switch value.(type) {
case string:
_ = flagset.StringP(flg, shorthand, vip.GetString(prefix), usage)

10
cli/deployment/config_test.go
View File

@ -199,6 +199,16 @@ func TestConfig(t *testing.T) {
Regex: "gitlab.com",
}}, config.GitAuth.Value)
},
}, {
Name: "Wrong env must not break default values",
Env: map[string]string{
"CODER_PROMETHEUS_ENABLE": "true",
"CODER_PROMETHEUS": "true", // Wrong env name, must not break prom addr.
},
Valid: func(config *codersdk.DeploymentConfig) {
require.Equal(t, config.Prometheus.Enable.Value, true)
require.Equal(t, config.Prometheus.Address.Value, config.Prometheus.Address.Default)
},
}} {
tc := tc
t.Run(tc.Name, func(t *testing.T) {