Unmarshal non-bound environment variables

[sagikazarmark]

This is going to be an umbrella issue for other open issues for this problem.

These are:

• Unmarshal() does not use the env variable if the config file has no that key #1236
• Commented out property in YAML yields inconsistencies between Get and Unmarshal when overriding using environment variables #725
• Unmarshal using ENV without binding #688
• Does viper require a config file in order to use ENV variables? #584
• Can I tell viper.Unmarshal to take environment variables into account? #212
• Consider env vars when unmarshalling #188
• Unmarshal doesn't check environment variables with AutomaticEnv #140

Problem

Currently Viper cannot unmarshal automatically loaded environment variables.

Here is a failing test case reproducing the issue:

package viper

import (
	"os"
	"testing"
)

func TestUnmarshal_AutomaticEnv(t *testing.T) {
	os.Setenv("MYKEY", "myvalue")

	type Config struct {
		MyKey string `mapstructure:"mykey"`
	}

	var c Config

	AutomaticEnv()

	if err := Unmarshal(&c); err != nil {
		t.Fatal(err)
	}

	if c.MyKey != "myvalue" {
		t.Error("failed to unmarshal automatically loaded environment variable")
	}

	os.Clearenv()
}

Root cause

Automatic environment variable loading works as follows:

1. User requests a value from Viper: viper.GetString("mykey")
2. Viper runs the env key replacer on the key
3. Viper compares the key (case insensitively) with the list of environment variables
4. Returns the value if one is found

Unmarshal, however, doesn't work by requesting a key, quite the opposite: it tries to unmarshal existing keys onto the given structure. Since automatically loaded environment variables are not in the list of existing keys (unlike defaults and bind env vars), these variables won't be unmarshaled.

Workaround

1. Use BindEnv to manually bind environment variables
2. Use SetDefault to set a default value (even an empty one), so Viper can find the key and try to find a matching env var

Possible solutions

The linked issues list a number of workarounds to fix this issue.

Personally, I'm in favor of the following:

1. Unmarshal the given struct onto a map[string]interface{}. That will give use a nested structure of all the keys otherwise mapstructure expects.
2. Flatten the structure, so we get a list of expected keys
3. Check that every key is defined. If not, check if there is a matching environment variable.
4. Merge the available settings with the matching, automatically loaded environment variables.
5. Use mapstructure as usual

Although I think that this is currently the best available solution, it doesn't mean we can't do better, so please do suggest if you have an idea.

[ktong]

Below is a wapper to unmarshal config with environment variables.

func Unmarshal(v View, key string, rawVal interface{}) error {
	cfg := v.(*viper.Viper)
	bindEnvs(cfg, key, rawVal)

	return cfg.UnmarshalKey(key, rawVal)
}

// Workaround because viper does not treat env vars the same as other config.
// See https://github.com/spf13/viper/issues/761.
func bindEnvs(cfg *viper.Viper, key string, rawVal interface{}) {
	for _, k := range allKeys(key, rawVal) {
		val := cfg.Get(k)
		cfg.Set(k, val)
	}
}

func allKeys(key string, rawVal interface{}) []string {
	b, err := yaml.Marshal(
		map[string]interface{}{
			key: rawVal,
		},
	)
	if err != nil {
		return nil
	}

	v := viper.New()
	v.SetConfigType("yaml")
	if err := v.ReadConfig(bytes.NewReader(b)); err != nil {
		return nil
	}

	return v.AllKeys()
}

[celian-garcia]

Hi guys, do you have a plan to support it ? I struggled with this as well and I used a solution inspired from one given by @krak3n but supporting pointers and ,squash feature of mapstructure.

Would be cool if mapstructure was exposing the feature of giving a tree of names based on the mapstructure tags. Then it would be easy to browse the tree and call the viper.BindEnv function with all the leaves.

For the moment, here is the solution I mentioned:

// bindenv: workaround to make the unmarshal work with environment variables
// Inspired from solution found here : https://github.com/spf13/viper/issues/188#issuecomment-399884438
func (b *viperConfigBuilder) bindenvs(iface interface{}, parts ...string) {
	ifv := reflect.ValueOf(iface)
	if ifv.Kind() == reflect.Ptr {
		ifv = ifv.Elem()
	}
	for i := 0; i < ifv.NumField(); i++ {
		v := ifv.Field(i)
		t := ifv.Type().Field(i)
		tv, ok := t.Tag.Lookup("mapstructure")
		if !ok {
			continue
		}
		if tv == ",squash" {
			b.bindenvs(v.Interface(), parts...)
			continue
		}
		switch v.Kind() {
		case reflect.Struct:
			b.bindenvs(v.Interface(), append(parts, tv)...)
		default:
			b.v.BindEnv(strings.Join(append(parts, tv), "."))
		}
	}
}

[iamolegga]

Hi all, in case someone need ready-to-use-package for this feature, I've just created one based on @celian-garcia and @krak3n examples, thank you guys!

[celian-garcia]

Thanks @iamolegga, I will use it :)

[segevfiner]

If mapstructure either returned metadata about fields not unmarshalled into in the target struct (Rather then the source), or otherwise had a callback to handle such fields, it could have been an easy way for viper to fix this. Otherwise using reflection in a similar way to what mapstructure does to handle the struct seems like the most viable approach, though it will have to correctly handle the various mapstructure tags. You can find various snippets of code doing just that around the issue tracker and the internet. Even then, it still won't work with dynamic parts of the struct, e.g. map[string]interface{} and so on, without further cleverness from viper rather then it just doing something akin to BindEnv.

Serializing the struct won't work correctly with omitempty.

[ricoleabricot]

I've also use a custom method for binding env vars without config file nor flags like so:

import (
	"github.com/fatih/structs"
	"github.com/jeremywohl/flatten"
)

func SetupConfigFromEnv() error {
	// Setup automatic env binding
	viper.AutomaticEnv()
	viper.SetEnvPrefix("")
	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))

	// Transform config struct to map
	confMap := structs.Map(Config{})

	// Flatten nested conf map
	flat, err := flatten.Flatten(confMap, "", flatten.DotStyle)
	if err != nil {
		return errors.Wrap(err, "Unable to flatten config")
	}

	// Bind each conf fields to environment vars
	for key, _ := range flat {
		err := viper.BindEnv(key)
		if err != nil {
			return errors.Wrapf(err, "Unable to bind env var: %s", key)
		}
	}

	return nil
}

It minimises "home-made" custom code but it has to use 2 other librairies to work.

[Blquinn]

@sagikazarmark Any news on this? Is it just a matter of a pull request?

[nikunjy]

Any update here ? Having to use BindEnv doesn't make sense. I have to write a constant in the mapstructure and specify the same string in BindEnv