config: Support ${FOO:bar}

[abhinav]

Our configuration system needs to support environment variables with defaults. ${FOO:bar} says that use the value of that environment variable here, and if the environment variable is unset, use bar.

One way to do this is to add support for it in the internal/decode package. The underlying library (mapstructure) allows registering hooks for type conversion. These hooks have access to the type you are converting from and to as well as the value you're converting. This is actually how the decoder.Decoder interface support is implemented.

So someone would have to write a hook that checks if the type you're converting from is a string, and the value is in the format specified above. Then, replace its contents and let the usual type conversion logic do the rest. I'm not sure about this but you may have to implement the WeaklyTypedInputs option in mapstructure to allow converting strings to ints.

[ghost]

FX already has this type of interpolation: https://github.com/uber-go/fx/pull/384, so may be it can be reused for parsing/loading to avoid duplication? This would require to move config outside of FX, though.

Also can it may help to simplify pluggable config...

[abhinav]

Unfortunately, YARPC cannot use UberFx's implementation because we need to support interpolating anywhere inside a parsed map[string]interface{} because being able to accept an arbitrary map[string]interface{} is part of YARPC's configuration API.

Quoting my comment from #888 which would originally have been posted here:

We want YARPC's configuration system to support this feature regardless since it's possible to use YARPC and the configuration system directly. We have existing customers who are using YARPC directly and relying on this functionality, and we need to continue supporting them. Additionally, we can't use UberFx's implementation because it processes []bytes before unmarshalling them. YARPC needs to support interpolating into strings anywhere inside the parsed map[string]interface{} after it has already been parsed from YAML.

[ghost]

Config is independent from other packages from UberFx, so using it with YARPC should be easy.

I don't quite understand, why processing []byte before unmarshalling is an issue? If this is because we do lookups only for YAMLs, then this is not an issue, we can easily make a wrapper for all config providers.

There are a few subtle differences between current UberFx and proposed YARPC behavior which would make debugging very hard for users if they'd migrate between YARPC and UberFx.

[HelloGrayson]

Regardless, I don't want YARPC taking a dependency on UberFX, and we need to support this feature even when the platform is being used directly.

[ghost]

@breerly I am not talking about YARPC taking dependency on UberFx, but forward users to do complex configuration with UberFx, instead of implementing it.

[abhinav]

@alsamylkin The configuration interface for YARPC is LoadConfig(serviceName string, v interface{}) where v is either a map[string]interface{} or map[interface{}]interface{}. YARPC accepts an arbitrary configuration map that is not necessarily YAML and builds up the result from that. This allows UberFx to do something like,

var yarpcCfg map[string]interface{}
yaml.Unmarshal(&yarpcCfg, bytes)
configurator.LoadConfig("myservice", yarpcConfig)

By the time LoadConfig is called, the YAML has already been unmarhsalled so we need to interpolate the variables into that. YARPC doesn't have access to the raw bytes from this entry point.

[ghost]

Ok, so this is an implementation detail, but I still don't quite understand why YARPC should implement this functionality, UberFx already supports it and we can extend support outside YAML.

[abhinav]

@alsamylkin My other comment already answers that but to summarize I'll try to list the questions I think you're asking and their answers.

• Why is YARPC adding this feature at all, instead of relying on UberFx to take care of it at their layer?

  We want YARPC's configuration system to support this feature regardless since it's possible to use YARPC and the configuration system directly. We have existing customers who are using YARPC directly and relying on this functionality, and we need to continue supporting them.

• Why is YARPC writing a custom implementation instead of copying UberFx's implementation?

  UberFx's implementation does not satisfy our requirements. We need to support interpolating inside arbitrary map[string]interface{}s, while UberFx's implementation interpolates at the []byte-level before unmarshalling.

• Why does YARPC need to support interpolating into map[string]interface{}?

  The YARPC configuration system has two entry points for users to provide configuration data:

  LoadConfigFromYAML(serviceName string, yamlData []byte)
  LoadConfig(serviceName string, data interface{})

  In the latter case, YARPC will receive an arbitrary map[string]interface{} or map[interface{}]interface{} parsed by the YAML/JSON/TOML library. This map will be decoded into a transport-specific configuration structure (like HTTPInboundConfig) depending on the TransportSpec. We need to apply interpolation before decoding into the transport-specific configuration structure but AFTER decoding the map[string]interface{} from YAML.

[bufdev]

Is this done now?

[abhinav]

Yeah