diff --git a/cli/deployment/config.go b/cli/deployment/config.go index ab0c0645cea45..1b32a36d6efb8 100644 --- a/cli/deployment/config.go +++ b/cli/deployment/config.go @@ -239,6 +239,7 @@ func newConfig() *codersdk.DeploymentConfig { Name: "Telemetry Enable", Usage: "Whether telemetry is enabled or not. Coder collects anonymized usage data to help improve our product.", Flag: "telemetry", + Env: "CODER_TELEMETRY", // Override default environment. Default: flag.Lookup("test.v") == nil, }, Trace: &codersdk.DeploymentConfigField[bool]{ @@ -294,6 +295,7 @@ func newConfig() *codersdk.DeploymentConfig { Name: "Trace Enable", Usage: "Whether application tracing data is collected. It exports to a backend configured by environment variables. See: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md", Flag: "trace", + Env: "CODER_TRACE", // Override default environment. }, HoneycombAPIKey: &codersdk.DeploymentConfigField[string]{ Name: "Trace Honeycomb API Key", @@ -370,7 +372,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") @@ -385,6 +386,18 @@ func Config(flagset *pflag.FlagSet, vip *viper.Viper) (*codersdk.DeploymentConfi return dc, nil } +func setEnv(prefix string, vip *viper.Viper, val reflect.Value) { + env := val.FieldByName("Env").String() + if env == "" { + env = formatEnv(prefix) + } + // Ensure the Env field is always populated. + val.FieldByName("Env").SetString(env) + + // Ensure the auto-generated or manually named env is bound. + vip.MustBindEnv(prefix, env) +} + func setConfig(prefix string, vip *viper.Viper, target interface{}) { val := reflect.Indirect(reflect.ValueOf(target)) typ := val.Type() @@ -393,21 +406,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: + setEnv(prefix, vip, val) val.FieldByName("Value").SetString(vip.GetString(prefix)) case bool: + setEnv(prefix, vip, val) val.FieldByName("Value").SetBool(vip.GetBool(prefix)) case int: + setEnv(prefix, vip, val) val.FieldByName("Value").SetInt(int64(vip.GetInt(prefix))) case time.Duration: + setEnv(prefix, vip, val) val.FieldByName("Value").SetInt(int64(vip.GetDuration(prefix))) case []string: + setEnv(prefix, vip, val) // 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 +440,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 +490,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 +526,6 @@ func NewViper() *viper.Viper { dc := newConfig() vip := viper.New() vip.SetEnvPrefix("coder") - vip.AutomaticEnv() vip.SetEnvKeyReplacer(strings.NewReplacer("-", "_", ".", "_")) setViperDefaults("", vip, dc) @@ -560,12 +583,21 @@ func setFlags(prefix string, flagset *pflag.FlagSet, vip *viper.Viper, target in if flg == "" { return } + + env := val.FieldByName("Env").String() + if env == "" { + env = formatEnv(prefix) + } usage := val.FieldByName("Usage").String() - usage = fmt.Sprintf("%s\n%s", usage, cliui.Styles.Placeholder.Render("Consumes $"+formatEnv(prefix))) + usage = fmt.Sprintf("%s\n%s", usage, cliui.Styles.Placeholder.Render("Consumes $"+env)) shorthand := val.FieldByName("Shorthand").String() 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, env) + switch value.(type) { case string: _ = flagset.StringP(flg, shorthand, vip.GetString(prefix), usage) diff --git a/cli/deployment/config_test.go b/cli/deployment/config_test.go index d86c8e06a29b6..52cae36e17165 100644 --- a/cli/deployment/config_test.go +++ b/cli/deployment/config_test.go @@ -35,7 +35,7 @@ func TestConfig(t *testing.T) { "CODER_PROVISIONER_DAEMONS": "5", "CODER_SECURE_AUTH_COOKIE": "true", "CODER_SSH_KEYGEN_ALGORITHM": "potato", - "CODER_TELEMETRY": "false", + "CODER_TELEMETRY": "true", "CODER_TELEMETRY_TRACE": "false", "CODER_WILDCARD_ACCESS_URL": "something-wildcard.com", }, @@ -50,7 +50,7 @@ func TestConfig(t *testing.T) { require.Equal(t, config.ProvisionerDaemons.Value, 5) require.Equal(t, config.SecureAuthCookie.Value, true) require.Equal(t, config.SSHKeygenAlgorithm.Value, "potato") - require.Equal(t, config.Telemetry.Enable.Value, false) + require.Equal(t, config.Telemetry.Enable.Value, true) require.Equal(t, config.Telemetry.Trace.Value, false) require.Equal(t, config.WildcardAccessURL.Value, "something-wildcard.com") }, @@ -117,7 +117,7 @@ func TestConfig(t *testing.T) { }, { Name: "Trace", Env: map[string]string{ - "CODER_TRACE_ENABLE": "true", + "CODER_TRACE": "true", "CODER_TRACE_HONEYCOMB_API_KEY": "my-honeycomb-key", }, Valid: func(config *codersdk.DeploymentConfig) { @@ -199,6 +199,23 @@ 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) + }, + }, { + Name: "Env fields are populated with manual or automatic name", + Valid: func(config *codersdk.DeploymentConfig) { + require.Equal(t, config.Trace.Enable.Env, "CODER_TRACE") // Manual + require.Equal(t, config.Telemetry.Enable.Env, "CODER_TELEMETRY") // Manual + require.Equal(t, config.Prometheus.Address.Env, "CODER_PROMETHEUS_ADDRESS") // Automatic + }, }} { tc := tc t.Run(tc.Name, func(t *testing.T) { diff --git a/codersdk/deploymentconfig.go b/codersdk/deploymentconfig.go index a73385670575a..248da8ee23bda 100644 --- a/codersdk/deploymentconfig.go +++ b/codersdk/deploymentconfig.go @@ -131,6 +131,7 @@ type DeploymentConfigField[T Flaggable] struct { Name string `json:"name"` Usage string `json:"usage"` Flag string `json:"flag"` + Env string `json:"env"` Shorthand string `json:"shorthand"` Enterprise bool `json:"enterprise"` Hidden bool `json:"hidden"` @@ -146,6 +147,7 @@ func (f *DeploymentConfigField[T]) MarshalJSON() ([]byte, error) { Name string `json:"name"` Usage string `json:"usage"` Flag string `json:"flag"` + Env string `json:"env"` Shorthand string `json:"shorthand"` Enterprise bool `json:"enterprise"` Hidden bool `json:"hidden"` @@ -156,6 +158,7 @@ func (f *DeploymentConfigField[T]) MarshalJSON() ([]byte, error) { Name: f.Name, Usage: f.Usage, Flag: f.Flag, + Env: f.Env, Shorthand: f.Shorthand, Enterprise: f.Enterprise, Hidden: f.Hidden, diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 764aab5c6e761..9f613ba4cf5d6 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -310,6 +310,7 @@ export interface DeploymentConfigField { readonly name: string readonly usage: string readonly flag: string + readonly env: string readonly shorthand: string readonly enterprise: boolean readonly hidden: boolean